Bug 13393 - Android.Accounts.AccountAuthenticatorActivity.Finish() binding missing
Summary: Android.Accounts.AccountAuthenticatorActivity.Finish() binding missing
Status: RESOLVED INVALID
Alias: None
Product: Android
Classification: Xamarin
Component: Bindings ()
Version: 4.8.x
Hardware: PC Windows
: --- normal
Target Milestone: ---
Assignee: Atsushi Eno
URL:
Depends on:
Blocks:
 
Reported: 2013-07-22 18:20 UTC by Jonathan M. Lane
Modified: 2013-07-24 15:05 UTC (History)
2 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED INVALID

Description Jonathan M. Lane 2013-07-22 18:20:35 UTC
While attempting to use the AccountAuthenticatorActivity base class to implement an Account login activity, I noticed the finish() method was not implemented in the Xamarin.Android.dll binding for this class. While not a critical method, adding this would allow Xamarin.Android users to remain consistent with the established Android usage of this base class and would end the potential need to re-implement the equivalent logic in the inheriting activities.

See: http://developer.android.com/reference/android/accounts/AccountAuthenticatorActivity.html#finish()
Comment 1 Atsushi Eno 2013-07-22 18:33:48 UTC
It does not have to exist because Finish() is Activity's member. Moreover, adding extraneous members harms user experience by increasing assembly size.
Comment 2 Jonathan M. Lane 2013-07-22 19:13:26 UTC
Android.App.Activity.Finish() does not fully cover the functionality of a would-be Android.Accounts.AcccountAuthenticatorActivity.Finish(). See https://github.com/android/platform_frameworks_base/blob/master/core/java/android/accounts/AccountAuthenticatorActivity.java#L70 and now see https://github.com/android/platform_frameworks_base/blob/master/core/java/android/app/Activity.java#L4140.

Without Android.Accounts.AcccountAuthenticatorActivity.Finish() being accessible in Xamarin.Android, you are stuck re-implementing that class yourself in C# as you have to check whether your Intent provided an AccountAuthenticatorResponse and whether or not a Bundle was passed via SetAccountAuthenticatorResult() so that you can finalize the activity and return that Bundle or an error code to the caller. The base Android.App.Activity() does not handle any of that, as it is functionality specific to activities that act as account login dialogues.

I can respect the sentiment of not wanting to add extraneous members to the core libraries. With that in mind, there is no point in providing bindings to this class if either of the two public members needed to make use of the functionality it provides are missing.

Please reconsider your position once you've looked at the source code for AccountAuthenticatorActivity.java (it is quite small).

Thanks for your time and attention.
Comment 3 Atsushi Eno 2013-07-23 00:55:45 UTC
You have some wrong premise; Android.Accounts.AccountAuthenticatorActivity.Finish() in user code invokes android.accounts.AccountAuthenticatorActivity#finish() regardless of managed method existence.
Comment 4 Jonathan M. Lane 2013-07-23 14:15:18 UTC
It is not intuitive that the Android.App.Activity.Finish() method would dispatch calls to the native Android method on the bound version of its child class. Is this behaviour documented somewhere?
Comment 5 Jonathan Pryor 2013-07-23 15:13:59 UTC
> It is not intuitive that the Android.App.Activity.Finish() method would
> dispatch calls to the native Android method on the bound version of its child
> class.

I'm coming up at a loss for what this means, so I'll just brain dump and hope that answers your question.

1. android.app.Activity.finish() is not `final`, which means it's `virtual` in C# parlance.

2. android.accounts.AccountAuthenticatorActivity.finish() is thus an override, even if @Override isn't specified.

3. We do not bind method overrides unless the override changes visibility/etc.

Consequently Android.Accounts.AccountAuthenticatorActivity.Finish() isn't declared in the binding assembly, as it isn't necessary.

http://docs.xamarin.com/guides/android/advanced_topics/java_integration_overview/working_with_jni#5.6.1.method-binding

When you inherit AccountAuthenticatorActivity and you call Activity.Finish(), `GetType () == ThresholdType` will be false, so JNIEnv.CallNonvirtualIntMethod() will be invoked. Here, ThresholdType will be typeof(AccountAuthenticatorActivity) (and ThresholdClass will be the Java-side AccountAuthenticatorActivity.class value), and thus your Finish() call will invoke AccountsAuthenticatorActivity.Finish(), which is what you wanted.

In short, things Just Work as they currently are, no "re-implementing that class yourself in C#" is needed.
Comment 6 Jonathan M. Lane 2013-07-24 03:29:25 UTC
I apologize for being unclear in my last post. I misunderstood how the Xamarin bindings actually call native code. Thank you Eno and Jonathan for taking the time to educate me.

I assumed (incorrectly, as Eno pointed out) there was a one-to-one mapping of managed classes and methods to the native equivalent. This is in part due to not having an understanding of effective library binding strategies but also because of how the Xamarin API documentation is an exact copy of the Android documentation for the classes/methods in question. This lead me to believe that methods such as Android.App.Activity.Finish() were simply wrappers around android.app.Activity#finish().

May I suggest you consider adding something to the API documentation that would make this behaviour obvious to those consuming it who may not have an understanding of how the bindings work on overridden native methods?
Comment 7 Jonathan Pryor 2013-07-24 12:56:22 UTC
> May I suggest you consider adding something to the API documentation that would
> make this behaviour obvious to those consuming it who may not have an
> understanding of how the bindings work on overridden native methods?

Interesting idea, but I'm not sure what this would actually entail. Our bindings are intended to do The Right Thing™, and adding a paragraph to every method's documentation stating what is actually happening could be...distracting?
Comment 8 Jonathan M. Lane 2013-07-24 15:05:37 UTC
I don't think a full explanation of what is happening is required, as you are right, that would be distracting. A short message clarifying the binding method invokes the appropriate method on the ThresholdClass, regardless of how far down the inheritance tree that managed methods appears to live, would be sufficient from my perspective.

I appreciate that your bindings do "The Right Thing". This may be a pretty small edge case and further marginal due to my ignorance of how things work in your assemblies. Just consider this a vote for documented clarification in places where the managed API appears to be missing members found in the bound API.

And now I'll stop beating the dead horse!

Thanks again.