Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Java.Interop] Additional marshaler lookup #389

Merged
merged 2 commits into from
Oct 26, 2018
Merged

[Java.Interop] Additional marshaler lookup #389

merged 2 commits into from
Oct 26, 2018

Conversation

radekdoulik
Copy link
Member

Before going to use the proxy object marshaler, try to check the
implemented interfaces for custom marshaler.

It will be used in XA to marshal IJavaObject based objects, which do
not implement IJavaPeerable.

That will make it possible to fix
#388

@radekdoulik
[Java.Interop] Additional marshaler lookup
a6607e1 
Before going to use the proxy object marshaler, try to check the
implemented interfaces for custom marshaler.

It will be used in XA to marshal IJavaObject based objects, which do
not implement IJavaPeerable.

That will make it possible to fix with
dotnet#388
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this pull request Oct 26, 2018
@radekdoulik
[Mono.Android] Add IJavaObject custom marshaler
588c521 
Once dotnet/java-interop#389 is merged, it
should fix dotnet/java-interop#388

Example of updated marshalers:

    using Android.Runtime;
    using Java.Interop;
    using System;

    public static void n_setAdapter_Landroid_widget_ListAdapter_ (IntPtr __jnienv, IntPtr __this, IntPtr value)
    {
    	JniTransition jniTransition = new JniTransition (__jnienv);
    	JniRuntime runtime = default(JniRuntime);
    	try {
    		runtime = JniEnvironment.Runtime;
    		JniRuntime.JniValueManager valueManager = runtime.ValueManager;
    		valueManager.WaitForGCBridgeProcessing ();
    		AbsListView value2 = valueManager.GetValue<AbsListView> (__this);
    		IListAdapter listAdapter2 = value2.Adapter = Java.Interop.JavaConvert.FromJniHandle<IListAdapter> (value, JniHandleOwnership.DoNotTransfer);
    	} catch (Exception ex) when (runtime.ExceptionShouldTransitionToJni (ex)) {
    		jniTransition.SetPendingException (ex);
    	} finally {
    		jniTransition.Dispose ();
    	}
    }

and

    using Android.Runtime;
    using Java.Interop;
    using System;

    public static IntPtr n_getAdapter (IntPtr __jnienv, IntPtr __this)
    {
    	JniTransition jniTransition = new JniTransition (__jnienv);
    	JniRuntime runtime = default(JniRuntime);
    	try {
    		runtime = JniEnvironment.Runtime;
    		JniRuntime.JniValueManager valueManager = runtime.ValueManager;
    		valueManager.WaitForGCBridgeProcessing ();
    		AbsListView value = valueManager.GetValue<AbsListView> (__this);
    		IListAdapter adapter = value.Adapter;
    		return JNIEnv.ToLocalJniHandle (adapter);
    	} catch (Exception ex) when (runtime.ExceptionShouldTransitionToJni (ex)) {
    		jniTransition.SetPendingException (ex);
    		return default(IntPtr);
    	} finally {
    		jniTransition.Dispose ();
    	}
    	IntPtr intPtr = default(IntPtr);
    	return intPtr;
    }
jonpryor
jonpryor reviewed Oct 26, 2018
View reviewed changes
src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs Outdated

foreach (var iface in info.ImplementedInterfaces) {
var ifaceInfo = iface.GetTypeInfo ();
marshalerAttr = ifaceInfo.GetCustomAttribute<JniValueMarshalerAttribute> ();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually make sense? I'm not sure it does.

This states that an interface can contain a [JniValueMarshaler] custom attribute on interfaces -- which is probably a mistake, as we'll see! -- but what should happen if a type implements more than one interface, each of which has a custom marshaler?

[JniValueMarshaler (...)]
public interface IX {}

[JniValueMarshaler (...)]
public interface IY {}

public class Example : JavaObject, IX, IY {
  // ...
}

With this PR, we'll wind up using the custom marshaler from the first interface. Is this correct? Is this even sane?

I think we might instead want to prevent allowing interfaces to have custom marshalers, and just restrict to classes/structs/etc.

@jonpryor jonpryor merged commit ec2813a into dotnet:master Oct 26, 2018
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this pull request Oct 31, 2018
@radekdoulik
[Mono.Android] Add IJavaObject custom marshaler
f924ac2 
Once dotnet/java-interop#389 is merged, it
should fix dotnet/java-interop#388

Example of updated marshalers:

    using Android.Runtime;
    using Java.Interop;
    using System;

    public static void n_setAdapter_Landroid_widget_ListAdapter_ (IntPtr __jnienv, IntPtr __this, IntPtr value)
    {
    	JniTransition jniTransition = new JniTransition (__jnienv);
    	JniRuntime runtime = default(JniRuntime);
    	try {
    		runtime = JniEnvironment.Runtime;
    		JniRuntime.JniValueManager valueManager = runtime.ValueManager;
    		valueManager.WaitForGCBridgeProcessing ();
    		AbsListView value2 = valueManager.GetValue<AbsListView> (__this);
    		IListAdapter listAdapter2 = value2.Adapter = Java.Interop.JavaConvert.FromJniHandle<IListAdapter> (value, JniHandleOwnership.DoNotTransfer);
    	} catch (Exception ex) when (runtime.ExceptionShouldTransitionToJni (ex)) {
    		jniTransition.SetPendingException (ex);
    	} finally {
    		jniTransition.Dispose ();
    	}
    }

and

    using Android.Runtime;
    using Java.Interop;
    using System;

    public static IntPtr n_getAdapter (IntPtr __jnienv, IntPtr __this)
    {
    	JniTransition jniTransition = new JniTransition (__jnienv);
    	JniRuntime runtime = default(JniRuntime);
    	try {
    		runtime = JniEnvironment.Runtime;
    		JniRuntime.JniValueManager valueManager = runtime.ValueManager;
    		valueManager.WaitForGCBridgeProcessing ();
    		AbsListView value = valueManager.GetValue<AbsListView> (__this);
    		IListAdapter adapter = value.Adapter;
    		return JNIEnv.ToLocalJniHandle (adapter);
    	} catch (Exception ex) when (runtime.ExceptionShouldTransitionToJni (ex)) {
    		jniTransition.SetPendingException (ex);
    		return default(IntPtr);
    	} finally {
    		jniTransition.Dispose ();
    	}
    	IntPtr intPtr = default(IntPtr);
    	return intPtr;
    }
@github-actions github-actions bot locked and limited conversation to collaborators Apr 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jonpryor jonpryor jonpryor left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@radekdoulik @jonpryor