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.

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

[Android] Add ReactiveActivity ctors for JNI ownership transfer #841

Merged
merged 2 commits into from
Apr 29, 2015

Conversation

jonfuller
Copy link
Contributor

In some circumstances, the Xamarin Android framework already has a reference to an object, but needs to create it's peer MCW. In these instances, the constructor that takes (IntPtr, JniHandleOwnership) is called.

This is detailed here: Java Activation

Also, an SO answer from @jonpryor discussing this a bit: http://stackoverflow.com/a/10603714/335675

This change adds the aforementioned constructors to both the ReactiveActivity and ReactiveActivity<TViewModel> classes.

@anaisbetts
Copy link
Member

Into this, do we need this for Fragments too?

@jonpryor
Copy link

Into this, do we need this for Fragments too?

Probably. Fragments can be created by Android via Reflection unless you set Fragment.RetainInstance to true. If Android is creating your Fragment instance first, you need the Fragment(IntPtr, JniHandleOwnership) constructor.

Also, be very careful when using generic fragments. Creating instances of generic managed types from Java is not supported.

@jonfuller
Copy link
Contributor Author

Good idea.

I can go ahead and add those constructors for the fragment classes as well, if that helps things out.

Just let me know.

Thanks!

@anaisbetts
Copy link
Member

@jonfuller Please do!

@jonfuller
Copy link
Contributor Author

Fragment constructors added! Thanks for the input/feedback @paulcbetts and @jonpryor .

anaisbetts added a commit that referenced this pull request Apr 29, 2015
[Android] Add ReactiveActivity ctors for JNI ownership transfer
@anaisbetts anaisbetts merged commit 8722733 into reactiveui:master Apr 29, 2015
@anaisbetts
Copy link
Member

Thanks @jonfuller!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants