[nativeaot] fix typemap logic involving duplicates

[jonathanpeppers]

70bd636 introduced an "MVP" for a managed typemap for NativeAOT.

Unfortunately, a dotnet new maui project template still crashes with:

AndroidRuntime: Process: net.dot.hellonativeaot, PID: 16075
AndroidRuntime: net.dot.jni.internal.JavaProxyThrowable: System.InvalidCastException: Arg_InvalidCastException
AndroidRuntime:    at Java.Lang.Object._GetObject[T](IntPtr, JniHandleOwnership) + 0x64
AndroidRuntime:    at Microsoft.Maui.WindowOverlay.Initialize() + 0x168
AndroidRuntime:    at Microsoft.Maui.Handlers.WindowHandler.OnRootViewChanged(Object sender, EventArgs e) + 0x8c
AndroidRuntime:    at Microsoft.Maui.Platform.NavigationRootManager.SetContentView(IView) + 0x17c
AndroidRuntime:    at Microsoft.Maui.Platform.NavigationRootManager.Connect(IView, IMauiContext) + 0xf0
AndroidRuntime:    at Microsoft.Maui.Handlers.WindowHandler.CreateRootViewFromContent(IWindowHandler, IWindow) + 0x4c
AndroidRuntime:    at Microsoft.Maui.Handlers.WindowHandler.MapContent(IWindowHandler handler, IWindow window) + 0x20
AndroidRuntime:    at Microsoft.Maui.PropertyMapper`2.<>c__DisplayClass5_0.<Add>b__0(IElementHandler h, IElement v) + 0x130
AndroidRuntime:    at Microsoft.Maui.PropertyMapper.UpdatePropertyCore(String, IElementHandler, IElement) + 0x58
AndroidRuntime:    at Microsoft.Maui.PropertyMapper.UpdateProperties(IElementHandler, IElement) + 0x74
AndroidRuntime:    at Microsoft.Maui.Handlers.ElementHandler.SetVirtualView(IElement) + 0x15c
AndroidRuntime:    at Microsoft.Maui.Controls.Element.SetHandler(IElementHandler) + 0x124
AndroidRuntime:    at Microsoft.Maui.Controls.Element.set_Handler(IElementHandler value) + 0xc
AndroidRuntime:    at Microsoft.Maui.Platform.ElementExtensions.SetHandler(Context, IElement, IMauiContext) + 0xfc
AndroidRuntime:    at Microsoft.Maui.Platform.ApplicationExtensions.CreatePlatformWindow(Activity, IApplication, Bundle) + 0x184
AndroidRuntime:    at Microsoft.Maui.MauiAppCompatActivity.OnCreate(Bundle) + 0x74
AndroidRuntime:    at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_savedInstanceState) + 0xc0
AndroidRuntime: at my.MainActivity.n_onCreate(Native Method)
AndroidRuntime: at my.MainActivity.onCreate(MainActivity.java:36)
AndroidRuntime: at android.app.Activity.performCreate(Activity.java:8960)
AndroidRuntime: at android.app.Activity.performCreate(Activity.java:8938)
AndroidRuntime: at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1526)
AndroidRuntime: at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3975)
AndroidRuntime: at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:4173)
AndroidRuntime: at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:114)
AndroidRuntime: at android.app.servertransaction.TransactionExecutor.executeNonLifecycleItem(TransactionExecutor.java:231)
AndroidRuntime: at android.app.servertransaction.TransactionExecutor.executeTransactionItems(TransactionExecutor.java:152)
AndroidRuntime: at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:93)
AndroidRuntime: at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2595)
AndroidRuntime: at android.os.Handler.dispatchMessage(Handler.java:107)
AndroidRuntime: at android.os.Looper.loopOnce(Looper.java:232)
AndroidRuntime: at android.os.Looper.loop(Looper.java:317)
AndroidRuntime: at android.app.ActivityThread.main(ActivityThread.java:8592)
AndroidRuntime: at java.lang.reflect.Method.invoke(Native Method)
AndroidRuntime: at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:580)
AndroidRuntime: at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:878)

One cause may be that Java.Interop.JavaObject is in the typemap
in favor of Java.Lang.Object! This could cause InvalidCastException.

The typemap implementation is missing two edge cases:

1. Types in Mono.Android.dll should be preferred over types in other
   assemblies.

2. Abstract or interface types should be preferred over their equivalent
   *Invoker types.

I fixed these cases in TypeMappingStep.cs and added a test that can
validate the typemap during a build.

[jonathanpeppers]

This code is trying to be the same as:

android/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs

Lines 314 to 321 in 0fb7b45

	if ((td.IsAbstract || td.IsInterface) &&
	!oldEntry.TypeDefinition.IsAbstract &&
	!oldEntry.TypeDefinition.IsInterface &&
	td.IsAssignableFrom (oldEntry.TypeDefinition, cache)) {
	// We found the `Invoker` type *before* the declared type
	// Fix things up so the abstract type is first, and the `Invoker` is considered a duplicate.
	duplicates.Insert (0, entry);
	oldEntry.SkipInJavaToManaged = false;

[jonpryor]

This will result in an ordering that is potentially different from what MonoVM has.

The problem is "aliases": Mono.Android.dll has them, too! For example, there are (at least) three bindings for java/util/ArrayList:

• Java.Util.ArrayList
• Android.Runtime.JavaList
• Android.Runtime.JavaList<T>

Which one "wins" currently? Android.Runtime.JavaList (non-generic). Why? TypeDefinition ordering within the assembly:

% monodis --typedef src/Mono.Android/obj/Debug/net10.0/android-35/Mono.Android.dll | grep 'Java.Util.ArrayList\|Android.Runtime.JavaList'
4951: Android.Runtime.JavaList (flist=36347, mlist=83422, flags=0x100001, extends=0x7c28)
4952: Android.Runtime.JavaList`1 (flist=36348, mlist=83468, flags=0x100001, extends=0x4d5c)
7399: Java.Util.ArrayList (flist=55415, mlist=134737, flags=0x100001, extends=0x7370)

Is this a problem? ¯\(ツ)/¯. It's just what it is now, and generally the default doesn't necessarily matter, because you'll be going through an Object.GetObject<T>() invocation which will specify the appropriate type.

Compare to what is done in the PR here: we have 3 candidate types, all in Mono.Android.dll, meaning we'll reverse the order. Thus, instead of preferring Android.Runtime.JavaList, this code will instead prefer Java.Util.ArrayList.

Again, is this a problem? I don't know, probably not? But it will be a change.

Is there a way to control the order of assemblies processed by the linker pipeline?

Looking at: https://github.com/dotnet/sdk/blob/a5393731b5b7b225692fff121f747fbbc9e8b140/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ILLink.targets#L124-L156

It feels like if we change @(ManagedAssemblyToLink) so that Mono.Android.dll is first, we'll get a similar result as what is done for MonoVM. Is it possible to reorder item group elements in that manner?

[jonathanpeppers]

I updated the logic, so these assertions work now:

AssertTypeMap ("java/util/ArrayList", "Android.Runtime.JavaList");
Assert.IsFalse (StringAssertEx.ContainsText (b.LastBuildOutput,
	"Duplicate typemap entry for java/util/ArrayList => Android.Runtime.JavaList`1"),
	"Should get log message about duplicate Android.Runtime.JavaList`1!");

They did not previously work, but now I think the logic for NativeAOT is the same as described above.

[ivanpovazan]

I think it would be beneficial to develop a more general solution, rather than one specific to NativeAOT.
We could reuse the same logic as it is done for NativeAOT, but instead enable it for all runtimes via feature switch (e.g., _AndroidUseManagedTypeMaps) moving the TypeMappings dictionary generation from runtime-specific *TypeManager to the base class.

_AndroidUseManagedTypeMaps feature would be off by default for Mono, and on for CoreCLR/NativeAOT.
Turning it on for Mono for testing purposes, would enable us to test the new mappings thoroughly especially for troubles with duplicates.
Making this a more general solution would prepare the code base for the solution with FrozenDictionary as we proceed forward with: dotnet/runtime#110691
Lastly, in case when the feature is turned off, we would have trimmed builds have the same behavior as before since codepaths guarded by the switch would be trimmed away.

@jonpryor @jonathanpeppers @vitek-karas what do you think ?

[jonathanpeppers]

@ivanpovazan I think the solution here could be adapted to CoreCLR pretty easily, and we don't need to support it on Mono. It is more of a temporary thing to get us by.

I thought that if dotnet/runtime#110691 were completed, it would be implemented inside the trimmer and ILC, and we could delete this trimmer step?

[vitek-karas]

I absolutely agree that we should try to use solutions which are the same for all runtimes. I would even say that it "should" work on mono (unless there's a good reason why it can't).

It's also true that the goal should be to get rid of all custom trimmer steps (if nothing else running them for NativeAOT is rather tricky and hacky).