Skip to content

Commit

Permalink
[trimming] preserve custom views and $(AndroidHttpClientHandlerType) (
Browse files Browse the repository at this point in the history
#8954)

Fixes: #8797

Here are two cases `TrimMode=full` can break applications:

* `$(AndroidHttpClientHandlerType)` set to a custom type

* Custom views (Android `.xml`) that are not referenced in C# code

In the `MarkJavaObjects` trimmer step we can preserve both of these
cases by:

* Passing in `$(AndroidHttpClientHandlerType)`, preserve the public,
  parameterless constructor of the type

* Pass in `$(_CustomViewMapFile)`, preserve `IJavaObject` types if
  they are found in the map file

Filed an issue to address the usage of `Foo` in tests in the future:

* #9008
  • Loading branch information
jonathanpeppers authored Jun 17, 2024
1 parent a954a33 commit bd95226
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 27 deletions.
53 changes: 53 additions & 0 deletions src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Mono.Linker;
using Mono.Linker.Steps;
using Mono.Tuner;
using Java.Interop.Tools.Cecil;
using Xamarin.Android.Tasks;

namespace MonoDroid.Tuner {
Expand All @@ -16,9 +17,46 @@ public class MarkJavaObjects : BaseMarkHandler
public override void Initialize (LinkContext context, MarkContext markContext)
{
base.Initialize (context, markContext);
context.TryGetCustomData ("AndroidHttpClientHandlerType", out string androidHttpClientHandlerType);
context.TryGetCustomData ("AndroidCustomViewMapFile", out string androidCustomViewMapFile);
var customViewMap = MonoAndroidHelper.LoadCustomViewMapFile (androidCustomViewMapFile);

markContext.RegisterMarkAssemblyAction (assembly => ProcessAssembly (assembly, androidHttpClientHandlerType, customViewMap));
markContext.RegisterMarkTypeAction (type => ProcessType (type));
}

bool IsActiveFor (AssemblyDefinition assembly)
{
return assembly.MainModule.HasTypeReference ("System.Net.Http.HttpMessageHandler") || assembly.MainModule.HasTypeReference ("Android.Util.IAttributeSet");
}

public void ProcessAssembly (AssemblyDefinition assembly, string androidHttpClientHandlerType, Dictionary<string, HashSet<string>> customViewMap)
{
if (!IsActiveFor (assembly))
return;

foreach (var type in assembly.MainModule.Types) {
// Custom HttpMessageHandler
if (!string.IsNullOrEmpty (androidHttpClientHandlerType) &&
androidHttpClientHandlerType.StartsWith (type.Name, StringComparison.Ordinal)) {
var assemblyQualifiedName = type.GetPartialAssemblyQualifiedName (Context);
if (assemblyQualifiedName == androidHttpClientHandlerType) {
Annotations.Mark (type);
PreservePublicParameterlessConstructors (type);
continue;
}
}

// Custom views in Android .xml files
if (!type.ImplementsIJavaObject (cache))
continue;
if (customViewMap.ContainsKey (type.FullName)) {
Annotations.Mark (type);
PreserveJavaObjectImplementation (type);
}
}
}

public void ProcessType (TypeDefinition type)
{
// If this isn't a JLO or IJavaObject implementer,
Expand Down Expand Up @@ -48,6 +86,21 @@ void PreserveJavaObjectImplementation (TypeDefinition type)
PreserveInterfaces (type);
}

void PreservePublicParameterlessConstructors (TypeDefinition type)
{
if (!type.HasMethods)
return;

foreach (var constructor in type.Methods)
{
if (!constructor.IsConstructor || constructor.IsStatic || !constructor.IsPublic || constructor.HasParameters)
continue;

PreserveMethod (type, constructor);
break; // We can stop when found
}
}

void PreserveAttributeSetConstructor (TypeDefinition type)
{
if (!type.HasMethods)
Expand Down
4 changes: 1 addition & 3 deletions src/Mono.Android/Android.Runtime/AndroidEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,7 @@ static IWebProxy GetDefaultProxy ()
[DynamicDependency (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor, typeof (Xamarin.Android.Net.AndroidMessageHandler))]
static object GetHttpMessageHandler ()
{
// FIXME: https://github.com/xamarin/xamarin-android/issues/8797
// Note that this is a problem for custom $(AndroidHttpClientHandlerType) or $XA_HTTP_CLIENT_HANDLER_TYPE
[UnconditionalSuppressMessage ("Trimming", "IL2057", Justification = "DynamicDependency should preserve AndroidMessageHandler.")]
[UnconditionalSuppressMessage ("Trimming", "IL2057", Justification = "Preserved by the MarkJavaObjects trimmer step.")]
[return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
static Type TypeGetType (string typeName) =>
Type.GetType (typeName, throwOnError: false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ _ResolveAssemblies MSBuild target.
;_OuterIntermediateAssembly=@(IntermediateAssembly)
;_OuterOutputPath=$(OutputPath)
;_OuterIntermediateOutputPath=$(IntermediateOutputPath)
;_OuterCustomViewMapFile=$(_CustomViewMapFile)
</_AdditionalProperties>
<_AndroidBuildRuntimeIdentifiersInParallel Condition=" '$(_AndroidBuildRuntimeIdentifiersInParallel)' == '' ">true</_AndroidBuildRuntimeIdentifiersInParallel>
</PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ This file contains the .NET 5-specific targets to customize ILLink
Used for the <ILLink CustomData="@(_TrimmerCustomData)" /> value:
https://github.com/dotnet/sdk/blob/a5393731b5b7b225692fff121f747fbbc9e8b140/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ILLink.targets#L147
-->
<_TrimmerCustomData Include="AndroidHttpClientHandlerType" Value="$(AndroidHttpClientHandlerType)" />
<_TrimmerCustomData Include="AndroidCustomViewMapFile" Value="$(_OuterCustomViewMapFile)" />
<_TrimmerCustomData Include="XATargetFrameworkDirectories" Value="$(_XATargetFrameworkDirectories)" />
<_TrimmerCustomData
Condition=" '$(_ProguardProjectConfiguration)' != '' "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,53 @@ static AssemblyDefinition CreateFauxMonoAndroidAssembly ()
return assm;
}

private void PreserveCustomHttpClientHandler (string handlerType, string handlerAssembly, string testProjectName, string assemblyPath)
private void PreserveCustomHttpClientHandler (
string handlerType,
string handlerAssembly,
string testProjectName,
string assemblyPath,
TrimMode trimMode)
{
var proj = new XamarinAndroidApplicationProject () { IsRelease = true };
testProjectName += trimMode.ToString ();

var class_library = new XamarinAndroidLibraryProject {
IsRelease = true,
ProjectName = "MyClassLibrary",
Sources = {
new BuildItem.Source ("MyCustomHandler.cs") {
TextContent = () => """
class MyCustomHandler : System.Net.Http.HttpMessageHandler
{
protected override Task <HttpResponseMessage> SendAsync (HttpRequestMessage request, CancellationToken cancellationToken) =>
throw new NotImplementedException ();
}
"""
},
new BuildItem.Source ("Bar.cs") {
TextContent = () => "public class Bar { }",
}
}
};
using (var libBuilder = CreateDllBuilder ($"{testProjectName}/{class_library.ProjectName}")) {
Assert.IsTrue (libBuilder.Build (class_library), $"Build for {class_library.ProjectName} should have succeeded.");
}

var proj = new XamarinAndroidApplicationProject {
ProjectName = "MyApp",
IsRelease = true,
TrimModeRelease = trimMode,
Sources = {
new BuildItem.Source ("Foo.cs") {
TextContent = () => "public class Foo : Bar { }",
}
}
};
proj.AddReference (class_library);
proj.AddReferences ("System.Net.Http");
string handlerTypeFullName = string.IsNullOrEmpty(handlerAssembly) ? handlerType : handlerType + ", " + handlerAssembly;
proj.SetProperty (proj.ActiveConfigurationProperties, "AndroidHttpClientHandlerType", handlerTypeFullName);
proj.MainActivity = proj.DefaultMainActivity.Replace ("base.OnCreate (bundle);", "base.OnCreate (bundle);\nvar client = new System.Net.Http.HttpClient ();");
using (var b = CreateApkBuilder (testProjectName)) {
using (var b = CreateApkBuilder ($"{testProjectName}/{proj.ProjectName}")) {
Assert.IsTrue (b.Build (proj), "Build should have succeeded.");

using (var assembly = AssemblyDefinition.ReadAssembly (Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, assemblyPath))) {
Expand All @@ -173,12 +212,14 @@ private void PreserveCustomHttpClientHandler (string handlerType, string handler
}

[Test]
public void PreserveCustomHttpClientHandlers ()
public void PreserveCustomHttpClientHandlers ([Values (TrimMode.Partial, TrimMode.Full)] TrimMode trimMode)
{
PreserveCustomHttpClientHandler ("Xamarin.Android.Net.AndroidMessageHandler", "",
"temp/PreserveAndroidMessageHandler", "android-arm64/linked/Mono.Android.dll");
"temp/PreserveAndroidMessageHandler", "android-arm64/linked/Mono.Android.dll", trimMode);
PreserveCustomHttpClientHandler ("System.Net.Http.SocketsHttpHandler", "System.Net.Http",
"temp/PreserveSocketsHttpHandler", "android-arm64/linked/System.Net.Http.dll");
"temp/PreserveSocketsHttpHandler", "android-arm64/linked/System.Net.Http.dll", trimMode);
PreserveCustomHttpClientHandler ("MyCustomHandler", "MyClassLibrary",
"temp/MyCustomHandler", "android-arm64/linked/MyClassLibrary.dll", trimMode);
}

[Test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,24 @@ public static bool ExistsInFrameworkPath (string assembly)
.Select (p => Path.GetDirectoryName (p.TrimEnd (Path.DirectorySeparatorChar)))
.Any (p => assembly.StartsWith (p, StringComparison.OrdinalIgnoreCase));
}

static readonly char [] CustomViewMapSeparator = [';'];

public static Dictionary<string, HashSet<string>> LoadCustomViewMapFile (string mapFile)
{
var map = new Dictionary<string, HashSet<string>> ();
if (!File.Exists (mapFile))
return map;
foreach (var s in File.ReadLines (mapFile)) {
var items = s.Split (CustomViewMapSeparator, count: 2);
var key = items [0];
var value = items [1];
HashSet<string> set;
if (!map.TryGetValue (key, out set))
map.Add (key, set = new HashSet<string> ());
set.Add (value);
}
return map;
}
}
}
14 changes: 1 addition & 13 deletions src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -408,19 +408,7 @@ public static Dictionary<string, HashSet<string>> LoadCustomViewMapFile (IBuildE
var cachedMap = engine?.GetRegisteredTaskObjectAssemblyLocal<Dictionary<string, HashSet<string>>> (mapFile, RegisteredTaskObjectLifetime.Build);
if (cachedMap != null)
return cachedMap;
var map = new Dictionary<string, HashSet<string>> ();
if (!File.Exists (mapFile))
return map;
foreach (var s in File.ReadLines (mapFile)) {
var items = s.Split (new char [] { ';' }, count: 2);
var key = items [0];
var value = items [1];
HashSet<string> set;
if (!map.TryGetValue (key, out set))
map.Add (key, set = new HashSet<string> ());
set.Add (value);
}
return map;
return LoadCustomViewMapFile (mapFile);
}

public static bool SaveCustomViewMapFile (IBuildEngine4 engine, string mapFile, Dictionary<string, HashSet<string>> map)
Expand Down
12 changes: 7 additions & 5 deletions tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Diagnostics.CodeAnalysis;
using Android.App;
using Android.App;
using Android.Content;
using Android.Util;
using Android.Views;
Expand All @@ -12,9 +11,14 @@ namespace Xamarin.Android.RuntimeTests
[TestFixture]
public class CustomWidgetTests
{
public CustomWidgetTests()
{
// FIXME: https://github.com/xamarin/xamarin-android/issues/9008
new Foo ();
}

// https://bugzilla.xamarin.com/show_bug.cgi?id=23880
[Test]
[DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (CustomTextView))]
public void UpperCaseCustomWidget_ShouldNotThrowInflateException ()
{
Assert.DoesNotThrow (() => {
Expand All @@ -24,7 +28,6 @@ public void UpperCaseCustomWidget_ShouldNotThrowInflateException ()
}

[Test]
[DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (CustomTextView))]
public void LowerCaseCustomWidget_ShouldNotThrowInflateException ()
{
Assert.DoesNotThrow (() => {
Expand All @@ -34,7 +37,6 @@ public void LowerCaseCustomWidget_ShouldNotThrowInflateException ()
}

[Test]
[DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (CustomTextView))]
public void UpperAndLowerCaseCustomWidget_FromLibrary_ShouldNotThrowInflateException ()
{
Assert.DoesNotThrow (() => {
Expand Down
4 changes: 4 additions & 0 deletions tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
namespace Mono.Android_Test.Library;

// FIXME: https://github.com/xamarin/xamarin-android/issues/9008
public class Foo { }

0 comments on commit bd95226

Please sign in to comment.