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

[trimming] preserve custom views and $(AndroidHttpClientHandlerType) #8954

Merged
merged 6 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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");
Copy link
Member Author

Choose a reason for hiding this comment

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

The point of this, is we don't want to iterate over every type in every assembly.

Is Android.Util.IAttributeSet required for any custom view?

https://github.com/xamarin/xamarin-android/blob/14b70ac32b2a9efefae127795e96ef73124c8557/tests/Mono.Android-Tests/Mono.Android-Test.Library/CustomTextView.cs#L9

Copy link
Member

Choose a reason for hiding this comment

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

See:

IAttributeSet is used so that the View can obtain any XML attributes specified on the view. The above stack overflow answer has a good example of this:

<com.anjithsasindran.RectangleView
    app:radiusDimen="5dp"
    app:rectangleBackground="@color/yellow"
    app:circleBackground="@color/green" />

The AttributeSet is how the RectangleView constructor would lookup the values for @app:radiusDimen, @app:rectangleBackground, and @app:circleBackground.

I believe, but cannot quickly verify, that the (Context, IAttributeSet) constructor is always used/preferred by Android.

}

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
jonathanpeppers marked this conversation as resolved.
Show resolved Hide resolved
if (!type.ImplementsIJavaObject (cache))
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we "register" mapFile? (And why didn't we do this before, given that the beginning of this method calls .GetRegisteredTaskObjectAssemblyLocal<…>(…)?)

var map  = LoadCustomViewMapFile (mapFile);
if (map != null) {
    engine.RegisterTaskObject (mapFile, map, RegisteredTaskObjectLifetime.Build, allowEarlyCollection: false);
}
return map;

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh the code changed... ah cos of the linker can't use RegisterTaskObject

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry just actually read your comment (its early). The Registration of the file happens in SaveCustomViewMapFile.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the new method that doesn't support RegisterTaskObject is for the linker/trimmer step assembly. It doesn't have access to MSBuild APIs.

}

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 { }
Loading