Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Java.Interop] Type & Member Remapping Support (#936)
Context: #867 There are two scenarios for which a "generalized" type & member remapping solution would be useful: 1. Desugaring 2. Microsoft Intune MAM Note: this new support is only present when targeting .NET 6+, and will not (currently) be present in Classic Xamarin.Android. ~~ Desugaring ~~ Context: dotnet/android#4574 (comment) Context: dotnet/android#6142 (comment) The [Android Runtime][0] is responsible for running the Dalvik bytecode within e.g. `classes.dex`. The Android runtime and Dalvik bytecode formats have apparently changed over time in order to support newer Java language features, such as support for static interface methods: package org.webrtc; public /* partial */ interface EGLBase { public static EGLBase create() { /* … */} } Support for static interface methods was added in Java 8 and in Android v8.0 (API-26). If you want to use static interface methods on an Android device running API-25 or earlier, you must [desugar][1] the Java Bytecode. The result of [desugaring][2] the above `org.webrtc.EGLBase` type is that the static methods are moved to a *new* type, with a `$-CC` suffix, "as if" the Java code were instead: package org.webrtc; public /* partial */ interface EGLBase { // … } public final /* partial */ class EGLBase$-CC { public static EGLBase create { /* … */ } } While this works for pure Java code, this *does not work* with Xamarin.Android, because our bindings currently expect (require) that the types & members our binding process detects don't "move": // C# Binding for EGLBase namespace Org.Webrtc { public partial interface IEGLBase { private static readonly JniPeerMembers _members = new JniPeerMembers ("org/webrtc/EGLBase", typeof (IEGLBase)); public static IEGLBase Create() { const string __id = "create.()Lorg/webrtc/EglBase;" var __rm = _members.StaticMethods.InvokeObjectMethod (__id, null); return Java.Lang.Object.GetObject<IEGLBase>(__rm.Handle, JniHandleOwnership.TransferLocalRef); } } } The `new JniPeerMembers(…)` invocation will use `JNIEnv::FindClass()` to lookup the `org/webrtc/EGLBase` type, and `IEGLBase.Create()` will use `JNIEnv::GetStaticMethodID()` and `JNIEnv::CallStaticObjectMethod()` to lookup and invoke the `EGLBase.create()` method. However, when desugaring is used, *there is no* `EGLBase.create()` method. Consequently, in a "desugared" environment, `Java.Lang.NoSuchMethodError` will be thrown when `IEGLBase.Create()` is invoked, as `EGLBase.create()` doesn't exist; it "should" instead be looking for `EGLBase$-CC.create()`! Introduce partial support for this scenario by adding the new method: namespace Java.Interop { public partial class JniRuntime { public partial class JniTypeManager { public IReadOnlyList<string>? GetStaticMethodFallbackTypes (string jniSimpleReference) => GetStaticMethodFallbackTypesCore (jniSimpleReference); protected virtual IReadOnlyList<string>? GetStaticMethodFallbackTypesCore (string jniSimpleReference) => null; } } } `JniPeerMembers.JniStaticMethods.GetMethodInfo()` will attempt to lookup the static method, "as normal". If the method cannot be found, then `JniRuntime.JniTypeManager.GetStaticMethodFallbackTypes()` will be called, and we'll attempt to resolve the static method on each type returned by `GetStaticMethodFallbackTypes()`. If `GetStaticMethodFallbackTypes()` returns `null` or no fallback type provides the method, then a `NoSuchMethodError` will be thrown. TODO: Update xamarin-android to override `GetStaticMethodFallbackTypes()`, to return `$"{jniSimpleReference}$-CC"`. ~~ Microsoft Intune MAM ~~ Context: https://www.nuget.org/packages/Microsoft.Intune.MAM.Remapper.Tasks/ Context: https://docs.microsoft.com/en-us/mem/intune/developer/app-sdk-xamarin#remapper The [Microsoft Intune App SDK Xamarin Bindings][3] is in a similar-yet-different position: it involves Java & Dalvik Bytecode manipulation for various security purposes, and in order to make that work reasonably from Xamarin.Android, they *also* rewrite Xamarin.Android IL so that it's consistent with the manipulated Dalvik bytecode. See `readme.md` and `content/MonoAndroid10/remapping-config.json` within the [`Microsoft.Intune.MAM.Remapper.Tasks NuGet package`][4] for some additional details/tidbits such as: > This task operates on assemblies to replace the base type classes > which Microsoft Intune requires to implement its SDK (for example, > Intune requires using a `MAMActivity` in place of `Activity` and > methods such as `OnMAMCreate` instead of `OnCreate`). "ClassRewrites": [ { "Class": { "From": "android.app.Activity", "To": "com.microsoft.intune.mam.client.app.MAMActivity" }, "Methods": [ { "MakeStatic": false, "OriginalName": "onCreate", "NewName": "onMAMCreate", "OriginalParams": [ "android.os.Bundle" ] } ] }, { "Class": { "From": "android.content.pm.PackageManager", "To": "com.microsoft.intune.mam.client.content.pm.MAMPackageManagement" }, "Methods": [ { "MakeStatic": true, "OriginalName": "checkPermission", } ] } ] "GlobalMethodCalls": [ { "Class": { "From": "android.app.PendingIntent", "To": "com.microsoft.intune.mam.client.app.MAMPendingIntent" }, "Methods": [ { "MakeStatic": false, "OriginalName": "getActivities" }, ] } ] While what the InTune team has *works*, it suffers from a number of "workflow" problems, e.g. incremental builds are problematic (they might try to rewrite assemblies which were already rewritten), IL rewriting is *always* "fun", and if we change IL binding patterns, they'll need to support previous "binding styles" and newer styles. Introduce partial support for this scenario by adding the following members to `Java.Interop.JniRuntime.JniTypeManager`: namespace Java.Interop { public partial class JniRuntime { public struct ReplacementMethodInfo { public string? SourceJniType {get; set;} public string? SourceJniMethodName {get; set;} public string? SourceJniMethodSignature {get; set;} public string? TargetJniType {get; set;} public string? TargetJniMethodName {get; set;} public string? TargetJniMethodSignature {get; set;} public int? TargetJniMethodParameterCount {get; set;} public bool TargetJniMethodIsStatic {get; set;} } public partial class JniTypeManager { public string? GetReplacementType (string jniSimpleReference) => GetReplacementTypeCore (jniSimpleReference); protected virtual string? GetReplacementTypeCore (string jniSimpleReference) => null; public ReplacementMethodInfo? GetReplacementMethodInfo (string jniSimpleReference, string jniMethodName, string jniMethodSignature) => GetReplacementMethodInfoCore (jniSimpleReference, jniMethodName,jniMethodSignature); protected virtual ReplacementMethodInfo? GetReplacementMethodInfoCore (string jniSimpleReference, string jniMethodName, string jniMethodSignature) => null; } } } These new methods are used by `JniPeerMembers`. Consider "normal" usage of `JniPeerMembers`, within a binding: partial class Activity { static readonly JniPeerMembers _members = new XAPeerMembers (jniPeerTypeName:"android/app/Activity", managedPeerType:typeof (Activity)); } `JniRuntime.JniTypeManager.GetReplacementType()` allows "replacing" the `jniPeerTypeName` value with a "replacement" JNI type name. The "replacement" type will be used for all field and method lookups. This allows supporting the `ClassRewrites[0].Class.From` / `ClassRewrites[0].Class.To` semantics. `JniRuntime.JniTypeManager.GetReplacementMethodInfo()` is the key integration for all other "replacement" semantics. Given the source type, source method name, and source JNI signature, it is responsible for providing an *overriding* target type, target method name, and target JNI signature. For `ClassRewrites[0].Methods[0]`, this allows "renaming" `Activity.onCreate()` to `MAMActivity.onMAMCreate()`: var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo ( // Note: must match GetReplacementType() *output*, and since // `Activity` is mapped to `MAMActivity`… "com/microsoft/intune/mam/client/app/MAMActivity", "onCreate", "(Landroid/os/Bundle;)V" ); // is equivalent to: var r = new JniRuntime.ReplacementMethodInfo { SourceJniType = "com/microsoft/intune/mam/client/app/MAMActivity", // from input parameter SourceJniMethodName = "onCreate", // from input parameter SourceJniMethodSignature = "(Landroid/os/Bundle;)V", // from input parameter TargetJniType = "com/microsoft/intune/mam/client/app/MAMActivity", TargetJniMethodName = "onMAMCreate", TargetJniMethodSignature = null, // "use original value" TargetJniMethodParameterCount = null, // unchanged TargetJniMethodIsStatic = false, } `ClassRewrites[1].Methods[0]` is "weird", in that it has `MakeStatic: true`. The semantics for when `MakeStatic: true` exists is that the "original" method is an *instance* method, and the target method is a *`static`* method, *and* the `this` instance now becomes a *parameter* to the method. This is likewise supported via `JniRuntime.JniTypeManager.GetReplacementMethodInfo()`, and is identified by: 1. `ReplacementMethodInfo.TargetJniMethodParameterCount` being a non-`null` value, and 2. `ReplacementMethodInfo.TargetJniMethodIsStatic` is true. Thus, `ClassRewrites[1].Methods[0]` is akin to: var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo ( "android/content/pm/PackageManager", "checkPermission", "(Ljava/lang/String;Ljava/lang/String;)I" ); // is equivalent to: var r = new JniRuntime.ReplacementMethodInfo { SourceJniType = "android/content/pm/PackageManager", // from input parameter SourceJniMethodName = "checkPermission", // from input parameter SourceJniMethodSignature = "(Ljava/lang/String;Ljava/lang/String;)I", // from input parameter TargetJniType = "com/microsoft/intune/mam/client/content/pm/MAMPackageManagement", TargetJniMethodName = "CheckPermission", TargetJniMethodSignature = "(Landroid/content/pm/PackageManager;Ljava/lang/String;Ljava/lang/String;)I", // Note: `PackageManager` is inserted as new first parameter TargetJniMethodParameterCount = 3, TargetJniMethodIsStatic = true, } (Note that the subclass is responsible for providing this data.) `ReplacementMethodInfo.TargetJniMethodParameterCount` must be the number of parameters that the target method accepts. This is needed so that `JniPeerMembers.JniInstanceMethods.TryInvoke*StaticRedirect()` can appropriately reallocate the `JniArgumentValue*` array, so that the `this` parameter can be added to the beginning of the parameters. ~~ Overhead? ~~ All these extra invocations into `JniRuntime.JniTypeManager` imply additional overhead to invoke a Java method. However, this is all done during the *first* time a method is looked up, after which the `JniMethodInfo` instance is *cached* for a given (type, method, signature) tuple. To demonstrate overheads, `samples/Hello` has been updated to accept a new `-t` value; when provided, it invokes the `java.lang.Object.hashCode()` method 1 million times and calculates the average invocation overhead: % dotnet run --project samples/Hello -- -t Object.hashCode: 1000000 invocations. Total=00:00:00.5196758; Average=0.0005196758ms If we replace the .NET 6 `samples/Hello/bin/Debug/Java.Interop.dll` assembly with e.g. `bin/Debug/Java.Interop.dll`, we can compare to performance *without* these new changes, as the changes are only in the .NET 6 build: % \cp bin/Debug/Java.Interop{.dll,.pdb} samples/Hello/bin/Debug % dotnet samples/Hello/bin/Debug/Hello.dll -t Object.hashCode: 1000000 invocations. Total=00:00:00.5386676; Average=0.0005386676ms There is a fair bit of variation in `dotnet Hello.dll -t` output, but they're all roughly similar. There doesn't appear to be significant overhead for this particular test. ~~ Other Changes ~~ Cleaned up the `JniPeerMembers` constructors. The `Debug.Assert()` calls were duplicative and redundant. Replace the `Debug.Assert()` with `Debug.WriteLine()`. `Mono.Android.NET-Tests.apk` was running into an "unfixable" scenario: WARNING: ManagedPeerType <=> JniTypeName Mismatch! javaVM.GetJniTypeInfoForType(typeof(Android.Runtime.JavaList)).JniTypeName="java/util/ArrayList" != "java/util/List" This was because of [`Android.Runtime.JavaList`][5] using a `JniPeerMembers` for `List` while registering `ArrayList`, causing typemaps to associate `JavaList` with `ArrayList`: [Register ("java/util/ArrayList", DoNotGenerateAcw=true)] partial class JavaList { internal static readonly JniPeerMembers list_members = new XAPeerMembers ("java/util/List", typeof (JavaList), isInterface: true); } @jonpryor doesn't want to try fixing this right now; turning the assertion into a diagnostic message feels preferable. [0]: https://en.wikipedia.org/wiki/Android_Runtime [1]: https://developer.android.com/studio/write/java8-support [2]: https://developer.android.com/studio/write/java8-support-table [3]: https://docs.microsoft.com/en-us/mem/intune/developer/app-sdk-xamarin [4]: https://www.nuget.org/packages/Microsoft.Intune.MAM.Remapper.Tasks/ [5]: https://github.com/xamarin/xamarin-android/blob/b250c04b09a2b725336ae6d6c5693e0b9f37e4cc/src/Mono.Android/Android.Runtime/JavaList.cs#L9-L13
- Loading branch information