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

Add API Level 27. #980

Merged
merged 1 commit into from
Nov 18, 2017
Merged

Add API Level 27. #980

merged 1 commit into from
Nov 18, 2017

Conversation

atsushieno
Copy link
Contributor

@atsushieno atsushieno commented Oct 26, 2017

I cannot check if it builds by myself because I never get working build locally. make clean should never fail but it never got fixed. And asking to run git clean -xdf for developers is a huge no-go.

@atsushieno
Copy link
Contributor Author

dotnet/android-tools#13 is a blocker.

<Name>Oreo</Name>
<Level>27</Level>
<Id>27</Id>
<Stable>True</Stable>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be False, not True, as our binding is not stable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea why current MSBuild based solution requires duplicate edits. This should be automatically generated from build-tools/scripts/BuildEverything.mk.

@atsushieno atsushieno force-pushed the api-level-27 branch 2 times, most recently from f90ed6f to 4efec40 Compare October 26, 2017 13:04
@jonpryor
Copy link
Member

dotnet/android-tools#13 is a blocker.

No, it should not be a blocker. The entire point to PR #599 / 8942eca is to remove the need to update xamarin-android-tools/androidtools/etc. for testing new API levels.

Instead, the new architecture is for AndroidApiInfo.xml files located in $(TargetFrameworkRootPath) to provide the necessary mapping information, removing the need to maintain a static mapping except for "historical" purposes.

If this is not the case -- if updating xamarin-android-tools is still in fact required -- then the solution is not to add another mapping for API-27. Instead, we'd need to fix AndroidVersions so that the "historical" mapping information isn't required.

What error are you encountering that you believe that dotnet/android-tools#13 is required?

@atsushieno
Copy link
Contributor Author

Oh, then never mind, I never tried to build the changes without changing xamarin-android-tools. It should be ignorable then.

@jonpryor
Copy link
Member

Please rebase this PR on commit e824170 or later. e824170 through bd2640e have green Jenkins builds, while commit 5c750430 -- what this PR was based on -- is not green.


We will have to enable this line and bring back some sanity.
-->
<!-- attr path="/api/package[@name='android.graphics']/class[@name='Color']" name="managedName">AndroidColor</attr -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this line commented out? Why not enable it now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes generator generating broken code because it depends on "_Color" type!


<!-- android.graphics.Color started to be in actual use and that uncovered
Xamarin.Android design mistake above! We have to remove WallpaperColors API
until Xamarin.Android fixes their bogus API design. -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh...what? Adding Android.Graphics.Color as a struct may have been "wrong decision made back in 2010," but it's not one we can change until we break API compatibility, which isn't going to happen anytime soon.

As such, we need to find an alternative that doesn't involve API breakage. Is there something wrong with the commented-out <attr/> which renames android.graphics.Color to Android.Graphics.AndroidColor?

Even without that <attr/>, why can't android.app.WallpaperColors be bound? I don't see anything in there which would prevent it from being bound at all. (It might be useless, lacking the getPrimaryColor() and related methods, but the type itself could still exist.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As commented above, changing _Color to something like AndroidColor causes breakage that needs complicated generator fixes (compared to just bringing in API Level 27, and such "fixes" should not block "enable API Level 27" effort).

@atsushieno
Copy link
Contributor Author

Just to make sure, nothing further will be worked on until #982 gets merged (because api-27.xml.in is totally based on the generated file from the tool that the PR brings in).

@atsushieno atsushieno changed the title [WIP] Add API Level 27. Add API Level 27. Nov 15, 2017
@jonpryor
Copy link
Member

I thought this depended on PR #1027 in order to work as intended. Should this PR be rebased atop commit 3750fbf?

That aside, the current macOS+xbuild PR Build failed, because of API breakage:

ABI BREAK IN: Mono.Android.dll
<!-- start namespace Android.Icu.Text --> <div> 
<h2>Namespace Android.Icu.Text</h2>
<!-- start type StringPrepParseException --> <div>
<h3>Type Changed: Android.Icu.Text.StringPrepParseException</h3>
<p>Removed constructor:</p>

<pre>
	<span class='removed removed-constructor breaking' data-is-breaking>public StringPrepParseException (string, StringPrepParseExceptionError);</span>
</pre>

</div> <!-- end type StringPrepParseException -->

</div> <!-- end namespace Android.Icu.Text -->
<!-- start namespace Android.Views --> <div> 
<h2>Namespace Android.Views</h2>
<!-- start type WindowManagerLayoutParams --> <div>
<h3>Type Changed: Android.Views.WindowManagerLayoutParams</h3>
<p>Removed constructor:</p>

<pre>
	<span class='removed removed-constructor breaking' data-is-breaking>public WindowManagerLayoutParams (WindowManagerTypes, WindowManagerFlags);</span>
</pre>

</div> <!-- end type WindowManagerLayoutParams -->

</div> <!-- end namespace Android.Views -->
<!-- start namespace Android.Widget --> <div> 
<h2>Namespace Android.Widget</h2>
<!-- start type Spinner --> <div>
<h3>Type Changed: Android.Widget.Spinner</h3>
<p>Removed constructor:</p>

<pre>
	<span class='removed removed-constructor breaking' data-is-breaking>public Spinner (Android.Content.Context, Android.Util.IAttributeSet, int, SpinnerMode);</span>
</pre>

</div> <!-- end type Spinner -->

</div> <!-- end namespace Android.Widget -->

These are all constructor removals, and my best guess regarding their removal is that commit 3750fbf is required.

@atsushieno
Copy link
Contributor Author

It looks like the API breakage is real, specific to this PR. I'll take a look.

@atsushieno
Copy link
Contributor Author

build

@jonpryor jonpryor merged commit 237964a into dotnet:master Nov 18, 2017
jonpryor added a commit that referenced this pull request May 20, 2022
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/
simonrozsival pushed a commit to simonrozsival/xamarin-android that referenced this pull request May 31, 2022
Fixes: dotnet/java-interop#867

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

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

  * dotnet/java-interop@1f27ab55: [Java.Interop] Type & Member Remapping Support (dotnet#936)
  * dotnet/java-interop@02aa54e0: [Java.Interop.Tools.JavaCallableWrappers] marshal method decl types (dotnet#987)
  * dotnet/java-interop@e7bacc37: [ci] Update azure-pipelines.yaml to Pin .NET 6.0.202 (dotnet#986)
  * dotnet/java-interop@fb94d598: [Java.Interop.Tools.JavaCallableWrappers] Collect overriden methods (dotnet#985)
  * dotnet/java-interop@3fcce746: [Java.Interop.{Dynamic,Export}] Nullable Reference Type support (dotnet#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:
dotnet#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/
@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 2024
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