Skip to content

Commit

Permalink
[Java.Interop] Remove JniRuntime.JniValueManager.GetObject()
Browse files Browse the repository at this point in the history
Context: https://bugzilla.xamarin.com/show_bug.cgi?id=25443
Fixes: dotnet/java-interop#2
Fixes: dotnet/java-interop#3

The problem with JniRuntime.JniValueManager.GetObject() is that it was
constrained to only return IJavaPeerable instances.  This constraint
in turn necessitated the introduction of JniMarshal.GetValue<T>()
(commit 1e14a61) so that "anything" could be stored within a
JavaObjectArray<T>, proxying values if required.  This in turn led to
a dichotomy: should JniRuntime.GetObject<T>() be used, or should
JniMarshal.GetValue<T>() be used?

This dichotomy was partially resolved in commit 77a6bf8, which
removed JniMarshal.GetValue<T>() and added a
JniRuntime.JniValueManager.GetValue<T>() method, but that just
introduced a different dichotomy: should JniValueManager.GetObject()
be used or JniValueManager.GetValue()?

Simplify this mess: kill JniValueManager.GetObject() (and all
overloads), and instead rely on JniValueManager.GetValue().

Additionally, add JniValueManager.CreateValue() overloads (Issue #3)
and fix JniValueMarshaler<T>.CreateGenericValue() logic to *not*
lookup registered values, as per commit 77a6bf8:

> clean[] up JniValueMarshaler.Create*Value() methods to always
> create values when able, simplifying the logic of
> JniValueMarshaler implementations.

Cleaning up these semantics is necessary so that
JniValueManager.CreateValue() can be meaningful.  The only time that
JniValueManager.CreateValue() won't create a *new* value is if the
value is a proxy'd instance, at which point we're not going to muck
with it and any resulting bugs are Not Our Problem™.

The primary change of killing JniValueManager.GetObject() required a
few secondary changes:

  * JniValueManager.RegisterObject<T>() can no longer throw a
    NotSupportedException when attempting to create an alias, as
    (1) the NotSupportedException is a semantic change from
    Xamarin.Android that we can't accept -- aliases are a fact of
    life, and will be required for .JavaCast<T> (Issue #10) -- and
    (2) the check was breaking unit tests.
    We now generate a warning to the GREF log when aliases are
    created, similar in spirit to what Xamarin.Android does.

  * ProxyValueMarshaler.CreateGenericValue() now falls back to
    JniValueManager.CreateObjectWrapper() instead of returning `null`.
    This was needed so that ValueManager.GetValue<Exception>(...)
    would work.

  * For a "unified type system"-like experience,
    JniValueManager.CreateObjectWrapper() now maps certain types to
    corresponding types, e.g. object is mapped to JavaObject, and
    Exception is mapped to JavaException.  This is likewise to support
    ValueManager.GetValue<Exception>(...) around a Java instance.
  • Loading branch information
jonpryor committed Dec 7, 2015
1 parent 77a6bf8 commit c659341
Show file tree
Hide file tree
Showing 19 changed files with 308 additions and 190 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public override unsafe object Invoke (IJavaPeerable self, JniArgumentValue* argu
{
if (self == null) {
var h = members.InstanceMethods.StartCreateInstance (JniSignature, typeof (JavaInstanceProxy), arguments);
self = JniEnvironment.Runtime.ValueManager.GetObject<JavaInstanceProxy> (ref h, JniObjectReferenceOptions.CopyAndDispose);
self = JniEnvironment.Runtime.ValueManager.GetValue<JavaInstanceProxy> (ref h, JniObjectReferenceOptions.CopyAndDispose);
}
members.InstanceMethods.FinishCreateInstance (JniSignature, self, arguments);
return new DynamicJavaInstance (self);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ protected virtual void Dispose (bool disposing)

protected static object ToReturnValue (ref JniObjectReference handle, string signature, int n)
{
var instance = JniEnvironment.Runtime.ValueManager.GetObject (ref handle, JniObjectReferenceOptions.CopyAndDispose);
var instance = JniEnvironment.Runtime.ValueManager.GetValue<IJavaPeerable> (ref handle, JniObjectReferenceOptions.CopyAndDispose);
switch (signature [n]) {
case 'L':
return new DynamicJavaInstance (instance);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ static Expression GetThis (Expression vm, Type targetType, Expression context)
{
return Expression.Call (
Expression.Property (vm, "ValueManager"),
"GetObject",
"GetValue",
new[]{targetType},
context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public void CreateMarshalFromJniMethodExpression_InstanceAction ()
ExportTest __this;
__jvm = JniEnvironment.Runtime;
__this = __jvm.ValueManager.GetObject<ExportTest>(__context);
__this = __jvm.ValueManager.GetValue<ExportTest>(__context);
__this.InstanceAction();
}
catch (Exception __e)
Expand Down Expand Up @@ -304,7 +304,7 @@ public void CreateMarshalFromJniMethodExpression_FuncInt64 ()
long __mret;
__jvm = JniEnvironment.Runtime;
__this = __jvm.ValueManager.GetObject<ExportTest>(__context);
__this = __jvm.ValueManager.GetValue<ExportTest>(__context);
__mret = __this.FuncInt64();
__jret = __mret;
return __jret;
Expand Down Expand Up @@ -343,7 +343,7 @@ public void CreateMarshalFromJniMethodExpression_FuncIJavaObject ()
JavaObject __mret;
__jvm = JniEnvironment.Runtime;
__this = __jvm.ValueManager.GetObject<ExportTest>(__context);
__this = __jvm.ValueManager.GetValue<ExportTest>(__context);
__mret = __this.FuncIJavaObject();
__jret = References.NewReturnToJniRef(__mret);
return __jret;
Expand Down
6 changes: 0 additions & 6 deletions src/Java.Interop/Java.Interop/JavaArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,6 @@ internal static Exception CreateMarshalNotSupportedException (Type sourceType, T
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);
var array = value as TArray;
if (array != null) {
JniObjectReference.Dispose (ref reference, transfer);
return array.ToTargetType (targetType, dispose: false);
}
return creator (ref reference, transfer)
.ToTargetType (targetType, dispose: true);
}
Expand Down
13 changes: 0 additions & 13 deletions src/Java.Interop/Java.Interop/JavaPeerableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,5 @@ public static string GetJniTypeName (this IJavaPeerable self)
JniPeerMembers.AssertSelf (self);
return JniEnvironment.Types.GetJniTypeNameFromInstance (self.PeerReference);
}

internal static object GetValue (ref JniObjectReference handle, JniObjectReferenceOptions transfer, Type targetType)
{
return JniEnvironment.Runtime.ValueManager.GetObject (ref handle, transfer, targetType);
}

internal static JniObjectReference CreateLocalRef (object value)
{
var o = value as IJavaPeerable;
if (o == null || !o.PeerReference.IsValid)
return new JniObjectReference ();
return o.PeerReference.NewLocalRef ();
}
}
}
9 changes: 5 additions & 4 deletions src/Java.Interop/Java.Interop/JavaProxyObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,21 @@ public static JavaProxyObject GetProxy (object value)

static bool _Equals (IntPtr jnienv, IntPtr n_self, IntPtr n_value)
{
var self = JniEnvironment.Runtime.ValueManager.GetObject<JavaProxyObject> (n_self);
var value = JniEnvironment.Runtime.ValueManager.GetObject (n_value);
var self = (JavaProxyObject) JniEnvironment.Runtime.ValueManager.PeekObject (new JniObjectReference (n_self));
var r_value = new JniObjectReference (n_value);
var value = JniEnvironment.Runtime.ValueManager.GetValue (ref r_value, JniObjectReferenceOptions.Copy);
return self.Equals (value);
}

static int _GetHashCode (IntPtr jnienv, IntPtr n_self)
{
var self = JniEnvironment.Runtime.ValueManager.GetObject<JavaProxyObject> (n_self);
var self = (JavaProxyObject) JniEnvironment.Runtime.ValueManager.PeekObject (new JniObjectReference (n_self));
return self.GetHashCode ();
}

static IntPtr _ToString (IntPtr jnienv, IntPtr n_self)
{
var self = JniEnvironment.Runtime.ValueManager.GetObject<JavaProxyObject> (n_self);
var self = (JavaProxyObject) JniEnvironment.Runtime.ValueManager.PeekObject (new JniObjectReference (n_self));
var s = self.ToString ();
var r = JniEnvironment.Strings.NewString (s);
try {
Expand Down
Loading

0 comments on commit c659341

Please sign in to comment.