Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] MAM Member Remapping? (#6591)
Browse files Browse the repository at this point in the history
Fixes: dotnet/java-interop#867

Context: dotnet/java-interop@1f27ab5
Context: #6142 (comment)
Context: #7020

Changes: dotnet/java-interop@843f3c7...1f27ab5

  * dotnet/java-interop@1f27ab55: [Java.Interop] Type & Member Remapping Support (#936)
  * dotnet/java-interop@02aa54e0: [Java.Interop.Tools.JavaCallableWrappers] marshal method decl types (#987)
  * dotnet/java-interop@e7bacc37: [ci] Update azure-pipelines.yaml to Pin .NET 6.0.202 (#986)
  * dotnet/java-interop@fb94d598: [Java.Interop.Tools.JavaCallableWrappers] Collect overriden methods (#985)
  * dotnet/java-interop@3fcce746: [Java.Interop.{Dynamic,Export}] Nullable Reference Type support (#980)

~~ The Scenarios ~~

Our Java binding infrastructure involves looking up types and methods
via [JNI][0], and assumes that types and methods won't "move" in an
unexpected manner.  Methods which move from a subclass to a
superclass works transparently.  Methods which are moved to an
entirely different class causes us problems.

Case in point: [desugaring][0], which can *move* Java types to places
that our bindings don't expect.  For example, [`Arrays.stream(T[])`][1]
may be moved into a `DesugarArrays` class, or a default interface
method `Example.m()` may be moved into an `Example$-CC` type.
Java.Interop has not supported such changes, resulting in throwing a
`Java.Lang.NoSuchMethodError` on Android versions where methods are
not where we expect.

Additionally, the [InTune Mobile Application Management][3] team
needs a expanded type and member lookup mechanism in order to
simplify how they maintain their product.  Currently they make things
work by rewriting IL, which can be brittle.


~~ Build actions ~~

To improve support for this, dotnet/java-interop#936 introduces
new `virtual` methods into `Java.Interop.JniRuntime.JniTypeManager`
which are called as part of type and member lookup, allowing
`AndroidTypeManager` to participate in the type and member resolution
process.

`AndroidTypeManager` in turn needs to know what types and members can
be remapped, and what they should be remapped to.  Some of these can
be algorithmic, such as pre-pending `Desugar` or appending `$-CC` for
the Desugar case.

The InTune use case involves a table, contained within the
[Microsoft.Intune.MAM.Remapper.Tasks NuGet package][4].

Update `src/Xamarin.Android.Build.Tasks` to add a new
`@(_AndroidRemapMembers)` Build action.  This build action is not
externally supported; it's to help test the feature.  Files with this
build action are XML files which control type and member remapping:

	<replacements>
	  <replace-type
	      from="android/app/Activity"
	      to="com/microsoft/intune/mam/client/app/MAMActivity" />
	  <replace-method
	      source-type="com/microsoft/intune/mam/client/app/MAMActivity"
	      source-method-name="onCreate" source-method-signature="(Landroid/os/Bundle;)V"
	      target-type="com/microsoft/intune/mam/client/app/MAMActivity"
	      target-method-name="onMAMCreate" target-method-instance-to-static="false" />
	</replacements>

`//replacements/replace-method` is structured with each attribute
corresponding to a member on the `JniRuntime.ReplacementMethodInfo`
structure, in dotnet/java-interop@1f27ab55.

  * `//replace-method/@source-type` is
    `JniRuntime.ReplacementMethodInfo.SourceJniType`
  * `//replace-method/@source-method-name` is
    `JniRuntime.ReplacementMethodInfo.SourceJniMethodName`
  * `//replace-method/@source-method-signature` is
    `JniRuntime.ReplacementMethodInfo.SourceJniMethodSignature`
  * `//replace-method/@target-type` is
    `JniRuntime.ReplacementMethodInfo.TargetJniType`
  * `//replace-method/@target-method-name` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodName`
  * `//replace-method/@target-method-signature` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodSignature`
    This attribute is optional.
  * `//replace-method/@target-method-parameter-count` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodParameterCount`.
    This attribute is optional.
  * `//replace-method/@target-method-instance-to-static` is
    `JniRuntime.ReplacementMethodInfo.TargetJniMethodIsStatic`

`@source-type`, `@source-method-name`, and `@source-method-signature`
combined serve as a "key" for looking up the associated `@target-*`
information.

Update `src/Xamarin.Android.Build.Tasks` to add a new
`@(_AndroidMamMappingFile)` Build action.  This build action is not
externally supported; it's to help test the feature.  Files with this
build action are expected to be JSON documents which follow the
current conventions of `remapping-config.json`, within the
`Microsoft.Intune.MAM.Remapper.Tasks` NuGet package.  This build
action is not externally supported; this is currently for testing
purposes.  `@(_AndroidMamMappingFile)` files are processed at build
time into `@(_AndroidRemapMembers)` XML files.

During App builds, all `@(_AndroidRemapMembers)` files are merged
into an `@(AndrodAsset)` named `xa-internal/xa-mam-mapping.xml`.
This asset is opened and provided to `JNIEnv.Initialize()` as part of
native app startup.


~~ Putting it all together ~~

This will only work on .NET 7+.

App project has a `@(_AndroidRemapMembers)` file.  This item is
processed during App build, stored into the `.apk`, and read during
app startup on an Android device.

Given a Java binding such as:

	public partial class Activity {
	    static readonly JniPeerMembers _members = new XAPeerMembers ("android/app/Activity", typeof (Activity));
	}

when the `JniPeerMembers` constructor runs, it will call
`JniEnvironment.Runtime.TypeManager.GetReplacementType("android/app/Activity")`.
If `@(_AndroidRemapMembers)` is based on the InTune
`remapping-config.json` file, then `android/app/Activity` is mapped
to `com/microsoft/intune/mam/client/app/MAMActivity`, and
`JNIEnv::FindClass()` will be told to lookup `MAMActivity`, *not*
`Activity`.

*If `MAMActivity` can't be found*, e.g. you're testing this all out,
the app will ~immediately crash, as `MAMActivity` doesn't exist. 😅

If `MAMActivity` can be found, eventually `Activity.OnCreate()` will
need to be invoked:

	partial class Activity {
	    protected virtual unsafe void OnCreate (Android.OS.Bundle? savedInstanceState)
	    {
	        const string __id = "onCreate.(Landroid/os/Bundle;)V";
	        try {
	            JniArgumentValue* __args = stackalloc JniArgumentValue [1];
	            __args [0] = new JniArgumentValue ((savedInstanceState == null) ? IntPtr.Zero : ((global::Java.Lang.Object) savedInstanceState).Handle);
	            _members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, __args);
	        } finally {
	            global::System.GC.KeepAlive (savedInstanceState);
	        }
	    }
	}

`_members.InstanceMethods.InvokeVirtualVoidMethod()` will internally
make a call similar to:

	var r = JniEnvironment.Runtime.TypeManager.GetReplacementMethodInfo (
	    "com/microsoft/intune/mam/client/app/MAMActivity",
	    "onCreate",
	    "(Landroid/os/Bundle;)V"
	);

The data returned will be 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",    // from //replace-method/@target-type
	    TargetJniMethodName             = "onMAMCreate",                                        // from //replace-method/@target-method-name
	    TargetJniMethodSignature        = "(Landroid/os/Bundle;)V",                             // from input parameter, as signature didn't change
	    TargetJniMethodParameterCount   = 1,                                                    // computed based on signature
	    TargetJniMethodIsStatic         = false,                                                // from //replace-method/@target-method-instance-to-static
	}

This will allow `_members.InstanceMethods.InvokeVirtualVoidMethod()`
to instead resolve and invoke `MAMActivity.onMAMCreate()`.


~~ Tools ~~

`tools/remap-mam-json-to-xml` is added, and will process the InTune
JSON file into `@(_AndroidRemapMembers)` XML:

	$ dotnet run --project tools/remap-mam-json-to-xml  -- \
	  $HOME/.nuget/packages/microsoft.intune.mam.remapper.tasks/0.1.4635.1/content/MonoAndroid10/remapping-config.json
	<replacements>…


~~ Unit Tests ~~

`@(_AndroidRemapMembers)` usage is required by
`Mono.Android.NET-Tests.apk`, as `Java.Interop-Tests.dll` exercises
the type and member remapping logic.


~~ Unrelated `gref+` logging fixes ~~

When `debug.mono.log` = `gref+`, the app could crash:

	signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 0x79c045dead

This was likely because a constant string was provided to
`OSBridge::_write_stack_trace()`, which tried to write into the
constant string, promptly blowing things up.

Workaround: don't use `gref+` logging when a GC occurs?
(Fortunately, `gref+` logging isn't the default.)

Better workaround: Don't Do That™. Don't write to const strings.


~~ About `@(_AndroidRemapMembers)` Semantics… ~~

 1. Changing the Java hierarchy "requires" changing the managed
    hierarchy to mirror it.

    If we rename `Activity` to `RemapActivity` but *don't* change
    `MainActivity` to inherit the (bound!) `Example.RemapActivity`,
    the app *crashes*:

        JNI DETECTED ERROR IN APPLICATION: can't call void example.RemapActivity.onMyCreate(android.os.Bundle) on instance of example.MainActivity

    This can be "fixed" *without* changing the base class
    of `MainActivity` by instead changing the base class of the
    Java Callable Wrapper for `MainActivity` to `example.RemapActivity`.
    This can be done manually (just edit the files in `obj/…`!), but
    isn't really supported in "normal" xamarin-android usage
    (the next Clean will wipe your changes).

    Presumably InTune would make this Just Work by e.g. patching the
    `MainActivity.class` file.

 2. `/replacements/replace-type` interacts with
    `/replacements/replace-method`: at runtime, `//replace-type@from`
    *no longer exists*, meaning you ***cannot*** use that name
    in `//replace-method/@source-type` either!

    If `Activity` is remapped to `RemapActivity`, then *there is no*
    `Activity.onCreate()` method to similarly remap.  Instead, you
    need to specify `RemapActivity.onCreate()`.

    This warps the brain a bit.

    This:

        <replace-method
            source-type="example/RemapActivity"
            source-method-name="onCreate"
            target-type="example/RemapActivity"
            target-method-name="onMyCreate" target-method-instance-to-static="false" />

    not this:

        <replace-method
            source-type="android/app/Activity"
            source-method-name="onCreate"
            target-type="example/RemapActivity"
            target-method-name="onMyCreate" target-method-instance-to-static="false" />

 3. Don't intermix type renames with
    `/replace-method/@target-method-instance-to-static='true']`.
    It *can* be done, but also warps the brain.

    The deal with `@target-method-instance-to-static` is that it
    it changes the target method signature -- unless explicitly
    provided in `/replace-method/@target-method-signature` --
    so that the "source declaring type" is a prefix.

    Thus given

        <replace-method
            source-type="android/view/View"
            source-method-name="setOnClickListener"
            target-type="example/ViewHelper"
            target-method-name="mySetOnClickListener" target-method-instance-to-static="true" />

    we'll look for `ViewHelper.mySetOnClickListener(View, View.OnClickListener)`.

    If we renamed `View` to `MyView`, we would instead look for
    `ViewHelper.mySetOnClickListener(MyView, View.OnClickListener)`
    (note changed parameter type).

    This almost certainly *won't* work.


~~ InTune Integration Testing? ~~

For "more complete" InTune integration testing, one will want the
path to `remapping-config.json`, without hardcoding things.
This can be done with `%(PackageReference.GeneratePathProperty)`=True
and using `$(PkgMicrosoft_Intune_MAM_Remapper_Tasks)`:

	<ItemGroup>
	  <PackageReference
	      Include="Microsoft.Intune.MAM.Remapper.Tasks"
	      Version="0.1.4635.1"
	      IncludeAssets="none"
	      GeneratePathProperty="True"
	      ReferenceOutputAssembly="False"
	  />
	</ItemGroup>
	<Target Name="_AddMamFiles"
	    BeforeTargets="_AddAndroidCustomMetaData">
	  <ItemGroup>
	    <_AndroidMamMappingFile Include="$(PkgMicrosoft_Intune_MAM_Remapper_Tasks)/content/MonoAndroid10/remapping-config.json" />
	  </ItemGroup>
	</Target>

This is still fraught with some peril, as it likely also depends on
getting the right "inner" build, which may require using the plural
`$(TargetFrameworks)` property, not the singular `$(TargetFramework)`.
This might still be a useful start.


~~ TODO ~~

Optimize this mess:
#7020

[0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html
[1]: https://developer.android.com/studio/write/java8-support#library-desugaring
[2]: https://developer.android.com/reference/java/util/Arrays#stream(T[])
[3]: https://docs.microsoft.com/en-us/mem/intune/fundamentals/what-is-intune
[4]: https://www.nuget.org/packages/Microsoft.Intune.MAM.Remapper.Tasks/
  • Loading branch information
jonpryor authored May 20, 2022
1 parent 9fd37e3 commit f6f11a5
Show file tree
Hide file tree
Showing 35 changed files with 1,160 additions and 62 deletions.
12 changes: 12 additions & 0 deletions Documentation/workflow/DevelopmentTips.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@

Tips and tricks while developing Xamarin.Android.

# Run MSBuild-Based On-Device Unit Tests

The [`tests/MSBuildDeviceIntegration`](tests/MSBuildDeviceIntegration)
directory contains NUnit-based unit tests which need to run against an attached
Android device (hardware or emulator). There are *lots* of tests in here, and
running them all can take a significant amount of time.

If you need to run only *one* `[Test]` method, you can use
[`dotnet test --filter`](https://docs.microsoft.com/dotnet/core/testing/selective-unit-tests?pivots=mstest):

./dotnet-local.sh test bin/TestDebug/MSBuildDeviceIntegration/net6.0/MSBuildDeviceIntegration.dll --filter "Name~TypeAndMemberRemapping"

# Update directory

When a Xamarin.Android app launches on an Android device, and the app was
Expand Down
2 changes: 1 addition & 1 deletion external/Java.Interop
162 changes: 158 additions & 4 deletions src/Mono.Android/Android.Runtime/AndroidRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Linq;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.Versioning;
using System.Text;
using System.Threading;
using System.Reflection;
Expand Down Expand Up @@ -263,7 +264,11 @@ protected override IEnumerable<Type> GetTypesForSimpleReference (string jniSimpl
{
string? j = JNIEnv.TypemapManagedToJava (type);
if (j != null) {
return j;
return
#if NET
GetReplacementTypeCore (j) ??
#endif // NET
j;
}
if (JNIEnv.IsRunningOnDesktop) {
return JavaNativeTypeManager.ToJniName (type);
Expand All @@ -274,14 +279,163 @@ protected override IEnumerable<Type> GetTypesForSimpleReference (string jniSimpl
protected override IEnumerable<string> GetSimpleReferences (Type type)
{
string? j = JNIEnv.TypemapManagedToJava (type);
#if NET
j = GetReplacementTypeCore (j) ?? j;
#endif // NET
if (JNIEnv.IsRunningOnDesktop) {
string? d = JavaNativeTypeManager.ToJniName (type);
if (j != null && d != null) {
return new[]{j, d};
}
if (d != null) {
return new[]{d};
}
}
if (j != null) {
yield return j;
return new[]{j};
}
if (JNIEnv.IsRunningOnDesktop) {
yield return JavaNativeTypeManager.ToJniName (type);
return Array.Empty<string> ();
}

#if NET
protected override IReadOnlyList<string>? GetStaticMethodFallbackTypesCore (string jniSimpleReference)
{
ReadOnlySpan<char> name = jniSimpleReference;
int slash = name.LastIndexOf ('/');
var desugarType = new StringBuilder (jniSimpleReference.Length + "Desugar".Length);
if (slash > 0) {
desugarType.Append (name.Slice (0, slash+1))
.Append ("Desugar")
.Append (name.Slice (slash+1));
} else {
desugarType.Append ("Desugar").Append (name);
}

return new[]{
desugarType.ToString (),
$"{jniSimpleReference}$-CC"
};
}

protected override string? GetReplacementTypeCore (string jniSimpleReference)
{
if (JNIEnv.ReplacementTypes == null) {
return null;
}
if (JNIEnv.ReplacementTypes.TryGetValue (jniSimpleReference, out var v)) {
return v;
}
return null;
}

protected override JniRuntime.ReplacementMethodInfo? GetReplacementMethodInfoCore (string jniSourceType, string jniMethodName, string jniMethodSignature)
{
if (JNIEnv.ReplacementMethods == null) {
return null;
}
#if !STRUCTURED
if (!JNIEnv.ReplacementMethods.TryGetValue (CreateReplacementMethodsKey (jniSourceType, jniMethodName, jniMethodSignature), out var r) &&
!JNIEnv.ReplacementMethods.TryGetValue (CreateReplacementMethodsKey (jniSourceType, jniMethodName, GetMethodSignatureWithoutReturnType ()), out r) &&
!JNIEnv.ReplacementMethods.TryGetValue (CreateReplacementMethodsKey (jniSourceType, jniMethodName, null), out r)) {
return null;
}
ReadOnlySpan<char> replacementInfo = r;

var targetType = GetNextString (ref replacementInfo);
var targetName = GetNextString (ref replacementInfo);
var targetSig = GetNextString (ref replacementInfo);
var paramCountStr = GetNextString (ref replacementInfo);
var isStaticStr = GetNextString (ref replacementInfo);

int? paramCount = null;
if (!paramCountStr.IsEmpty) {
if (!int.TryParse (paramCountStr, 0, System.Globalization.CultureInfo.InvariantCulture, out var count)) {
return null;
}
paramCount = count;
}

bool isStatic = false;
if (isStaticStr.Equals ("true", StringComparison.Ordinal)) {
isStatic = true;
}

if (targetSig.IsEmpty && isStatic) {
paramCount = paramCount ?? JniMemberSignature.GetParameterCountFromMethodSignature (jniMethodSignature);
paramCount++;
jniMethodSignature = $"(L{jniSourceType};" + jniMethodSignature.Substring ("(".Length);
}

return new JniRuntime.ReplacementMethodInfo {
SourceJniType = jniSourceType,
SourceJniMethodName = jniMethodName,
SourceJniMethodSignature = jniMethodSignature,
TargetJniType = targetType.IsEmpty ? jniSourceType : new string (targetType),
TargetJniMethodName = targetName.IsEmpty ? jniMethodName : new string (targetName),
TargetJniMethodSignature = targetSig.IsEmpty ? jniMethodSignature : new string (targetSig),
TargetJniMethodParameterCount = paramCount,
TargetJniMethodInstanceToStatic = isStatic,
};
#else
if (!JNIEnv.ReplacementMethods.TryGetValue ((jniSourceType, jniMethodName, jniMethodSignature), out var r) &&
!JNIEnv.ReplacementMethods.TryGetValue ((jniSourceType, jniMethodName, GetMethodSignatureWithoutReturnType ()), out r) &&
!JNIEnv.ReplacementMethods.TryGetValue ((jniSourceType, jniMethodName, null), out r)) {
return null;
}
var targetSig = r.TargetSignature;
var paramCount = r.ParamCount;
if (targetSig == null && r.TurnStatic) {
targetSig = $"(L{jniSourceType};" + jniMethodSignature.Substring ("(".Length);
paramCount = paramCount ?? JniMemberSignature.GetParameterCountFromMethodSignature (jniMethodSignature);
paramCount++;
}
return new JniRuntime.ReplacementMethodInfo {
SourceJniType = jniSourceType,
SourceJniMethodName = jniMethodName,
SourceJniMethodSignature = jniMethodSignature,
TargetJniType = r.TargetType ?? jniSourceType,
TargetJniMethodName = r.TargetName ?? jniMethodName,
TargetJniMethodSignature = targetSig ?? jniMethodSignature,
TargetJniMethodParameterCount = paramCount,
TargetJniMethodInstanceToStatic = r.TurnStatic,
};
#endif // !STRUCTURED

string GetMethodSignatureWithoutReturnType ()
{
int i = jniMethodSignature.IndexOf (')');
return jniMethodSignature.Substring (0, i+1);
}

string GetValue (string? value)
{
return value == null ? "null" : $"\"{value}\"";
}

ReadOnlySpan<char> GetNextString (ref ReadOnlySpan<char> info)
{
int index = info.IndexOf ('\t');
var r = info;
if (index >= 0) {
r = info.Slice (0, index);
info = info.Slice (index+1);
return r;
}
info = default;
return r;
}
}

static string CreateReplacementMethodsKey (string? sourceType, string? methodName, string? methodSignature) =>
new StringBuilder ()
.Append (sourceType)
.Append ('\t')
.Append (methodName)
.Append ('\t')
.Append (methodSignature)
.ToString ();
#endif // NET

delegate Delegate GetCallbackHandler ();

static MethodInfo? dynamic_callback_gen;
Expand Down
19 changes: 19 additions & 0 deletions src/Mono.Android/Android.Runtime/JNIEnv.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@
using Java.Interop.Tools.TypeNameMappings;
using System.Diagnostics.CodeAnalysis;

#if NET
using ReplacementTypesDict = System.Collections.Generic.Dictionary<string, string>;
using ReplacementMethodsDict = System.Collections.Generic.Dictionary<string, string>;
#endif // NET

namespace Android.Runtime {
#pragma warning disable 0649
struct JnienvInitializeArgs {
Expand All @@ -35,6 +40,8 @@ struct JnienvInitializeArgs {
public int packageNamingPolicy;
public byte ioExceptionType;
public int jniAddNativeMethodRegistrationAttributePresent;
public IntPtr mappingXml;
public int mappingXmlLen;
}
#pragma warning restore 0649

Expand All @@ -61,6 +68,11 @@ public static partial class JNIEnv {
static AndroidRuntime? androidRuntime;
static BoundExceptionType BoundExceptionType;

#if NET
internal static ReplacementTypesDict? ReplacementTypes;
internal static ReplacementMethodsDict? ReplacementMethods;
#endif // NET

[ThreadStatic]
static byte[]? mvid_bytes;

Expand Down Expand Up @@ -166,6 +178,13 @@ internal static unsafe void Initialize (JnienvInitializeArgs* args)
gref_class = args->grefClass;
mid_Class_forName = new JniMethodInfo (args->Class_forName, isStatic: true);

#if NET
if (args->mappingXml != IntPtr.Zero) {
var xml = Encoding.UTF8.GetString ((byte*) args->mappingXml, args->mappingXmlLen);
(ReplacementTypes, ReplacementMethods) = MamXmlParser.ParseStrings (xml);
}
#endif // NET

if (args->localRefsAreIndirect == 1)
IdentityHash = v => _monodroid_get_identity_hash_code (Handle, v);
else
Expand Down
Loading

0 comments on commit f6f11a5

Please sign in to comment.