Update Mono.Android.xml to reflect all the changes in api-o-enumification branch

[atsushieno]

See dotnet/android#662 for all those details.
Particularly dotnet/android#662 (comment)
for the latest details.

[jonpryor]

This PR leaves one change of interest:

<h3>Type Changed: Android.Bluetooth.BluetoothGattServerCallback</h3>
<p>Removed method:</p>
<pre>
	<span class='removed removed-method breaking' data-is-breaking>public virtual void OnServiceAdded (ProfileState, BluetoothGattService);</span>
</pre>

I understand that this is a fix for Bug #55473, but it still constitutes an ABI break. Is there any way to work around this/provide ABI compatibility?

Case in point: searching GitHub for use of BluetoothGattServerCallback finds at least three results of interest:

• https://github.com/sbondini/AndroidBluetoothLE/blob/b23e64425634378ed4cf14da56cc1b5a113af6d4/AndroidBluetoothLE/Bluetooth/Server/GattServerObserver.cs#L43
• https://github.com/tboogh/ReactiveBluetooth/blob/9f71736db508dc534d767c328eae9fe56f4f5822/ReactiveBluetooth.Android/Peripheral/GattServer/ServerCallback.cs#L73
• https://github.com/chennanoka/CoAPNonIP_V1/blob/96d26b9b027589b55fd2f866f475f4685b19df29/CoAPNonIP.Core/LibCoAPNonIP/Network/Android/CallBack/NP2PSBLECallBack.cs#L219

Removing this member will break any existing assemblies with code like the above.

One not-fully-explored solution: "just" overload the method:

// Current
partial class BluetoothGattServerCallback {
    public virtual void OnServiceAdded(ProfileState, BluetoothGattService);
}

// Proposed
partial class BluetoothGattServerCallback {
    // New hotness
    public virtual void OnServiceAdded(GattStatus status, BluetoothGattService service);

    // Old and busted
    [Obsolete("Use OnServiceAdded(Android.Bluetooth.GattStatus, Android.Bluetooth.BluetoothGattService)", error:true)]
	[Register ("onServiceAdded", "(ILandroid/bluetooth/BluetoothGattService;)V", "GetOnServiceAdded_ILandroid_bluetooth_BluetoothGattService_Handler")]
    public virtual void OnServiceAdded(ProfileState status, BluetoothGattService service)
    {
        this.OnServiceAdded ((GattStatus)(int) status, service);
    }


	static Delegate cb_onServiceAdded_ILandroid_bluetooth_BluetoothGattService_;
#pragma warning disable 0169
	static Delegate GetOnServiceAdded_ILandroid_bluetooth_BluetoothGattService_Handler ()
	{
		if (cb_onServiceAdded_ILandroid_bluetooth_BluetoothGattService_ == null)
			cb_onServiceAdded_ILandroid_bluetooth_BluetoothGattService_ = JNINativeWrapper.CreateDelegate ((Action<IntPtr, IntPtr, int, IntPtr>) n_OnServiceAdded_ILandroid_bluetooth_BluetoothGattService_);
		return cb_onServiceAdded_ILandroid_bluetooth_BluetoothGattService_;
	}

	static void n_OnServiceAdded_ILandroid_bluetooth_BluetoothGattService_ (IntPtr jnienv, IntPtr native__this, int native_status, IntPtr native_service)
	{
		Android.Bluetooth.BluetoothGattServerCallback __this = global::Java.Lang.Object.GetObject<Android.Bluetooth.BluetoothGattServerCallback> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
		Android.Bluetooth.ProfileState status = (Android.Bluetooth.ProfileState) native_status;
		Android.Bluetooth.BluetoothGattService service = global::Java.Lang.Object.GetObject<Android.Bluetooth.BluetoothGattService> (native_service, JniHandleOwnership.DoNotTransfer);
		__this.OnServiceAdded (status, service);
	}
#pragma warning restore 0169
}

What I'm not currently sure about is runtime JNI method registration. I think it might be sufficient, because the [Obsolete(..., true)] will ensure that no derived types can override both OnServiceAdded() overloads.

[atsushieno]

I don't care about old API that is supposed to be fixed. As long as the new method in the API works it is basically no worse than "nothing else" so I have added the manual binding above with some fixes at d3f8957 and I'm going to update this repo too.

[jonpryor]

The updated commit confuses me; I'd expect BluetoothGattServerCallback to now have two OnServiceAdded() overloads, but the contents of the updated Mono.Android.xml only contains one OnServiceAdded() method.

Are you sure you updated reference/Mono.Android.xml after adding the new compatibility overload?

[atsushieno]

Sorry, that was unexpected. Updated.

[jonpryor]

Proposed commit message once merged:

---

There are three breaking changes in this update which we consider
to be acceptable:

1. Removal of Android.Service.Quicksettings.TileState
2. Removal of Java.Lang.Reflect.Constructor.InterfaceConsts
3. Removal of Java.Lang.Reflect.Method.InterfaceConsts

The TileState enum (1) is present due to a type-o: the namespace is
supposed to be Android.Service.QuickSettings (note the
capital-S in QuickSettings), and there is also an identical
Android.Service.QuickSettings.TileState enum type.

The incorrect Android.Service.Quicksettings.TileState type has been
[Obsolete] for quite some time, so we're removing it now.

The InterfaceConsts types (2, 3) are for static final int members
from a Java interface. (In this case, Constructor and Method are
classes, so the InterfaceConsts types are coming from interfaces
those types implement.)

Related: https://developer.xamarin.com/guides/android/advanced_topics/api_design/

All classes that implement a Java interface containing constants
get a new nested InterfaceConsts type which contains constants
from all implemented interfaces.

The Constructor.InterfaceConsts and Method.InterfaceConsts contained
the const members Declared and Public. Note that these are const,
so there is no runtime binding to them (outside Reflection). Also note
that the entire reason these constants existed is because Android was
originally based on Harmony OpenJDK, which contained these constants --
through java.lang.reflect.Member.DECLARED and
java.lang.reflect.Member.PUBLIC -- and Google has since removed these
constants in API-26.

[jonpryor]

However, while writing that up, the parts for (2) and (3) confuse me, as the rationale in that comment is wrong, at least locally: my android-26/android.jar does contain DECLARED and PUBLIC fields:

$ unzip android-26/android.jar java/lang/reflect/Member.class
$ mono /Library/Frameworks/Xamarin.Android.framework/Libraries/mandroid/class-parse.exe java/lang/reflect/Member.class --dump
...
Fields Count: 2
	0: DECLARED I Public, Static, Final
		ConstantValue(Integer(1))
	1: PUBLIC I Public, Static, Final
		ConstantValue(Integer(0))
...

So I no longer understand why e.g. Constructor.InterfaceConsts was removed. :-(

Perhaps I have an older android-26/android.jar?

[jonpryor]

Continuing my confusion, xamarin-android/src/Mono.Android/Profiles/api-26.xml.in contains:

    <interface abstract="true" deprecated="not deprecated" final="false" name="Member" static="false" visibility="public">
      <method abstract="true" deprecated="not deprecated" final="false" name="getDeclaringClass" native="false" return="java.lang.Class&lt;?&gt;" static="false" synchronized="false" visibility="public">
      </method>
      <method abstract="true" deprecated="not deprecated" final="false" name="getModifiers" native="false" return="int" static="false" synchronized="false" visibility="public">
      </method>
      <method abstract="true" deprecated="not deprecated" final="false" name="getName" native="false" return="java.lang.String" static="false" synchronized="false" visibility="public">
      </method>
      <method abstract="true" deprecated="not deprecated" final="false" name="isSynthetic" native="false" return="boolean" static="false" synchronized="false" visibility="public">
      </method>
      <field deprecated="not deprecated" final="true" name="DECLARED" static="true" transient="false" type="int" type-generic-aware="int" value="1" visibility="public" volatile="false">
      </field>
      <field deprecated="not deprecated" final="true" name="PUBLIC" static="true" transient="false" type="int" type-generic-aware="int" value="0" visibility="public" volatile="false">
      </field>
    </interface>

Confirming that these members have not in fact been removed from android.jar.

It's doubly odd that only Constructor and Method were changed in this fashion. It looks like this is because of their base class changing from java.lang.reflect.AccessibleObject (API-25)tojava.lang.reflect.Executable(API-26), and that base class change *only* happened withConstructorandMethod`.

Furthermore, as per the updated Mono.Android.xml, there is an Executable.InterfaceConsts type which contains the members Declared and Public.

So this looks like an acceptable change, but for different reasons: it isn't that java.lang.reflect.Member.DECLARED was removed; instead, it's that -- due to binding choices -- the declaring class for InterfaceConsts changed from being the class itself (Constructor.InterfaceConsts) to the new intermediate base class Executable.InterfaceConsts.

With this in mind, this is not an ABI break: it's not possible for the Constructor.InterfaceConsts type to appear in IL as-is: it's a static class, so it can't be created, and the only members it has are const fields, so there shouldn't be a memberref to those members either.

It's also not an API (source) break: the new Executable base class still contains an InterfaceConsts type, so the C# expression Constructor.InterfaceConsts.Default will continue to work, unchanged.

Consequently, the PR is correct. It's the commit message which requires editing.