[XABT] Move JLO scanning needed for typemap generation to a linker step (#10015)

Context: ea399ed
Context: 787a2a6
Context: b54ec05
COntext: b11471b

Move the process of scanning for `Java.Lang.Object` (JLO) and
`Java.Lang.Throwable` subclasses needed for the typemap generation
task to a new `FindTypeMapObjectsStep` "linker step".

The types are serialized to new `<assembly>.typemap.xml` files that
sit beside the source assembly.  A new file was used rather than adding
more sections to the `.jlo.xml` file because this file contains a lot
more types and will likely change more often on incremental builds, and
we don't want to trigger ie: creating new Java stubs when not needed.

This file is then read in the existing `<GenerateTypeMappings/>` task
to generate the final typemap file.

The existing method of JLO scanning for typemaps is still used when
using LLVM marshal methods, as the marshal method rewriter runs after
the linker steps, and when it changes the assemblies the MVID and type
tokens no longer match the values saved in the `.typemap.xml` files.

Like ea399ed, this temporarily leaves the old typemap generation code
in place, guarded behind the `$(_AndroidJLOCheckedBuild)` flag.
This flag generates the typemap both the new and old way, and errors
out if there are differences.

Note that b54ec05 updated our "pipeline" to save assemblies after
*all* steps have been run.  However, looking for JLOs needed for
typemap generation has to be run *after* assemblies are saved.
This is because Release typemaps rely on assembly MVID and type tokens
which change when the assemblies are modified.

Update the pipeline's "modified assembly saving" logic to itself be a
pipeline step, so we can insert it *before* the new
`FindTypeMapObjectsStep` step.

*Note*: previously, we scanned for all JLO subclasses from *all*
assemblies in a single loop, so [duplicate detection logic][0] finds
duplicates even if they are in different assemblies.

Consider `java.lang.Object`, which is bound as:

  * `Java.Lang.Object, Mono.Android`
  * `Java.Interop.JavaObject, Java.Interop`
  * *and also* several other unit test types as "aliases" (b11471b)

When processing `Java.Interop.dll`, the `TypeMapDebugEntry` fields
would be:

  - JavaName: "java/lang/Object" 
  - ManagedName: "Java.Interop.JavaObject, Java.Interop" 
  - SkipInJavaToManaged: "False" 
  - DuplicateForJavaToManaged
    - JavaName: "java/lang/Object" 
    - ManagedName: "Java.Lang.Object, Mono.Android" 

In particular, `TypeMapDebugEntry.DuplicateForJavaToManaged` mentions
`Java.Lang.Object, Mono.Android` as the "main" typemap.

*Now* scanning is performed *per-assembly*; it will find duplicates in
the same assembly, but not duplicates that may exist in other
assemblies.  The `TypeMapDebugEntry` fields when processing
`Java.Interop.dll` are instead:

  - JavaName: "java/lang/Object" 
  - ManagedName: "Java.Interop.JavaObject, Java.Interop" 
  - SkipInJavaToManaged: "False" 
  - DuplicateForJavaToManaged: null

Note that `TypeMapDebugEntry.DuplicateForJavaToManaged` is null.


We have two ordering semantics we have to preserve in replicating
the original logic:

  - A declared type should take precedence over an "invoker" type.
  - Types in `Mono.Android` have precedence over other assemblies.

Aside from these two cases, which type is considered the "primary"
and which type(s) are considered the "duplicates" seems to be luck
of the draw.

Note that "luck" only enters the picture when using the non-generic
`JniRuntime.JniValueManager.GetPeer(JniObjectReference, Type?)` API
and providing `null` for the `Type?` parameter.  Whenever a `Type`
*is provided*, or a generic wrapper such as
`Java.Lang.Object.GetObject<T>()` is used, then value returned is
*constrained* to the specified type, reducing the likelihood of
obtaining an unexpected type.

[0]: https://github.com/dotnet/android/blob/8c741263550cc57bafc30193fa61f39b7df019bc/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapCecilAdapter.cs#L141-L152

Author: jpobst

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/AddKeepAlivesStep.cs

@@ -29,13 +29,13 @@ protected override void ProcessAssembly (AssemblyDefinition assembly)
 		}
 
 #if !ILLINK
-		public bool ProcessAssembly (AssemblyDefinition assembly, StepContext context)
+		public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
 		{
 			// Only run this step on user Android assemblies
 			if (!context.IsAndroidUserAssembly)
-				return false;
+				return;
 
-			return AddKeepAlives (assembly);
+			context.IsAssemblyModified |= AddKeepAlives (assembly);
 		}
 #endif  // !ILLINK
 

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FindJavaObjectsStep.cs

@@ -29,19 +29,15 @@ public class FindJavaObjectsStep : BaseStep, IAssemblyModifierPipelineStep
 
 	public FindJavaObjectsStep (TaskLoggingHelper log) => Log = log;
 
-	public bool ProcessAssembly (AssemblyDefinition assembly, StepContext context)
+	public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
 	{
 		var destinationJLOXml = JavaObjectsXmlFile.GetJavaObjectsXmlFilePath (context.Destination.ItemSpec);
 		var scanned = ScanAssembly (assembly, context, destinationJLOXml);
 
 		if (!scanned) {
 			// We didn't scan for Java objects, so write an empty .xml file for later steps
 			JavaObjectsXmlFile.WriteEmptyFile (destinationJLOXml, Log);
-			return false;
 		}
-
-		// This step does not change the assembly
-		return false;
 	}
 
 	public bool ScanAssembly (AssemblyDefinition assembly, StepContext context, string destinationJLOXml)

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FindTypeMapObjectsStep.cs

@@ -0,0 +1,73 @@
+#nullable enable
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.Android.Build.Tasks;
+using Microsoft.Build.Utilities;
+using Mono.Cecil;
+using Mono.Linker.Steps;
+using Xamarin.Android.Tasks;
+
+namespace MonoDroid.Tuner;
+
+/// <summary>
+/// Scans an assembly for JLOs that need to be in the typemap and writes them to an XML file.
+/// </summary>
+public class FindTypeMapObjectsStep : BaseStep, IAssemblyModifierPipelineStep
+{
+	public bool Debug { get; set; }
+
+	public bool ErrorOnCustomJavaObject { get; set; }
+
+	public TaskLoggingHelper Log { get; set; }
+
+	public FindTypeMapObjectsStep (TaskLoggingHelper log) => Log = log;
+
+	public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
+	{
+		var destinationTypeMapXml = TypeMapObjectsXmlFile.GetTypeMapObjectsXmlFilePath (context.Destination.ItemSpec);
+
+		// We only care about assemblies that can contains JLOs
+		if (!context.IsAndroidAssembly) {
+			Log.LogDebugMessage ($"Skipping assembly '{assembly.Name.Name}' because it is not an Android assembly");
+			TypeMapObjectsXmlFile.WriteEmptyFile (destinationTypeMapXml, Log);
+			return;
+		}
+
+		var types = ScanForJavaTypes (assembly);
+
+		var xml = new TypeMapObjectsXmlFile {
+			AssemblyName = assembly.Name.Name,
+		};
+
+		if (Debug) {
+			var (javaToManaged, managedToJava, foundJniNativeRegistration) = TypeMapCecilAdapter.GetDebugNativeEntries (types, Context);
+
+			xml.JavaToManagedDebugEntries.AddRange (javaToManaged);
+			xml.ManagedToJavaDebugEntries.AddRange (managedToJava);
+			xml.FoundJniNativeRegistration = foundJniNativeRegistration;
+		} else {
+			var genState = TypeMapCecilAdapter.GetReleaseGenerationState (types, Context, out var foundJniNativeRegistration);
+			xml.ModuleReleaseData = genState.TempModules.SingleOrDefault ().Value;
+		}
+
+		xml.Export (destinationTypeMapXml, Log);
+	}
+
+	List<TypeDefinition> ScanForJavaTypes (AssemblyDefinition assembly)
+	{
+		var types = new List<TypeDefinition> ();
+
+		var scanner = new XAJavaTypeScanner (Xamarin.Android.Tools.AndroidTargetArch.None, Log, Context) {
+			ErrorOnCustomJavaObject = ErrorOnCustomJavaObject
+		};
+
+		foreach (ModuleDefinition md in assembly.Modules) {
+			foreach (TypeDefinition td in md.Types) {
+				scanner.AddJavaType (td, types);
+			}
+		}
+
+		return types;
+	}
+}

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs

@@ -83,13 +83,13 @@ internal bool FixAbstractMethods (AssemblyDefinition assembly)
 		}
 
 #if !ILLINK
-		public bool ProcessAssembly (AssemblyDefinition assembly, StepContext context)
+		public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
 		{
 			// Only run this step on non-main user Android assemblies
 			if (context.IsMainAssembly || !context.IsAndroidUserAssembly)
-				return false;
+				return;
 
-			return FixAbstractMethods (assembly);
+			context.IsAssemblyModified |= FixAbstractMethods (assembly);
 		}
 #endif  // !ILLINK
 

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixLegacyResourceDesignerStep.cs

@@ -71,13 +71,13 @@ protected override void LoadDesigner ()
 		}
 
 #if !ILLINK
-		public bool ProcessAssembly (AssemblyDefinition assembly, Xamarin.Android.Tasks.StepContext context)
+		public void ProcessAssembly (AssemblyDefinition assembly, Xamarin.Android.Tasks.StepContext context)
 		{
 			// Only run this step on non-main user Android assemblies
 			if (context.IsMainAssembly || !context.IsAndroidUserAssembly)
-				return false;
+				return;
 
-			return ProcessAssemblyDesigner (assembly);
+			context.IsAssemblyModified |= ProcessAssemblyDesigner (assembly);
 		}
 #endif  // !ILLINK
 

src/Xamarin.Android.Build.Tasks/Tasks/AssemblyModifierPipeline.cs

@@ -8,6 +8,7 @@
 using Java.Interop.Tools.TypeNameMappings;
 using Microsoft.Android.Build.Tasks;
 using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
 using Mono.Cecil;
 using MonoDroid.Tuner;
 using Xamarin.Android.Tools;
@@ -78,10 +79,6 @@ public override bool RunTask ()
 			ReadSymbols = ReadSymbols,
 		};
 
-		var writerParameters = new WriterParameters {
-			DeterministicMvid = Deterministic,
-		};
-
 		Dictionary<AndroidTargetArch, Dictionary<string, ITaskItem>> perArchAssemblies = MonoAndroidHelper.GetPerArchAssemblies (ResolvedAssemblies, Array.Empty<string> (), validate: false);
 
 		AssemblyPipeline? pipeline = null;
@@ -122,7 +119,7 @@ public override bool RunTask ()
 
 			Directory.CreateDirectory (Path.GetDirectoryName (destination.ItemSpec));
 
-			RunPipeline (pipeline!, source, destination, writerParameters);
+			RunPipeline (pipeline!, source, destination);
 		}
 
 		pipeline?.Dispose ();
@@ -141,9 +138,26 @@ protected virtual void BuildPipeline (AssemblyPipeline pipeline, MSBuildLinkCont
 
 		findJavaObjectsStep.Initialize (context);
 		pipeline.Steps.Add (findJavaObjectsStep);
+
+		// SaveChangedAssemblyStep
+		var writerParameters = new WriterParameters {
+			DeterministicMvid = Deterministic,
+		};
+
+		var saveChangedAssemblyStep = new SaveChangedAssemblyStep (Log, writerParameters);
+		pipeline.Steps.Add (saveChangedAssemblyStep);
+
+		// FindTypeMapObjectsStep - this must be run after the assembly has been saved, as saving changes the MVID
+		var findTypeMapObjectsStep = new FindTypeMapObjectsStep (Log) {
+			ErrorOnCustomJavaObject = ErrorOnCustomJavaObject,
+			Debug = Debug,
+		};
+
+		findTypeMapObjectsStep.Initialize (context);
+		pipeline.Steps.Add (findTypeMapObjectsStep);
 	}
 
-	void RunPipeline (AssemblyPipeline pipeline, ITaskItem source, ITaskItem destination, WriterParameters writerParameters)
+	void RunPipeline (AssemblyPipeline pipeline, ITaskItem source, ITaskItem destination)
 	{
 		var assembly = pipeline.Resolver.GetAssembly (source.ItemSpec);
 
@@ -157,17 +171,36 @@ void RunPipeline (AssemblyPipeline pipeline, ITaskItem source, ITaskItem destina
 			IsUserAssembly = ResolvedUserAssemblies.Any (a => a.ItemSpec == source.ItemSpec),
 		};
 
-		var changed = pipeline.Run (assembly, context);
+		pipeline.Run (assembly, context);
+	}
+}
+
+class SaveChangedAssemblyStep : IAssemblyModifierPipelineStep
+{
+	public TaskLoggingHelper Log { get; set; }
 
-		if (changed) {
-			Log.LogDebugMessage ($"Saving modified assembly: {destination.ItemSpec}");
-			Directory.CreateDirectory (Path.GetDirectoryName (destination.ItemSpec));
-			writerParameters.WriteSymbols = assembly.MainModule.HasSymbols;
-			assembly.Write (destination.ItemSpec, writerParameters);
+	public WriterParameters WriterParameters { get; set; }
+
+	public SaveChangedAssemblyStep (TaskLoggingHelper log, WriterParameters writerParameters)
+	{
+		Log = log;
+		WriterParameters = writerParameters;
+	}
+
+	public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
+	{
+		if (context.IsAssemblyModified) {
+			Log.LogDebugMessage ($"Saving modified assembly: {context.Destination.ItemSpec}");
+			Directory.CreateDirectory (Path.GetDirectoryName (context.Destination.ItemSpec));
+			WriterParameters.WriteSymbols = assembly.MainModule.HasSymbols;
+			assembly.Write (context.Destination.ItemSpec, WriterParameters);
 		} else {
 			// If we didn't write a modified file, copy the original to the destination
-			CopyIfChanged (source, destination);
+			CopyIfChanged (context.Source, context.Destination);
 		}
+
+		// We just saved the assembly, so it is no longer modified
+		context.IsAssemblyModified = false;
 	}
 
 	void CopyIfChanged (ITaskItem source, ITaskItem destination)