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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jonathanpeppers
Copy link
Member

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

Fixes: xamarin#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
@jonathanpeppers jonathanpeppers marked this pull request as ready for review May 17, 2024 13:44
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?

public CustomTextView (Context context, IAttributeSet attributes) : base (context, attributes)

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.

Comment on lines +15 to +16
// NOTE: ensure the C# compiler has a reference to the library
new Mono.Android_Test.Library.Foo ();
Copy link
Member Author

Choose a reason for hiding this comment

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

This was another problem, when we have a @(ProjectReference) to the class library containing Mono.Android_Test.Library -- there was no C# code using types from the library. And so the C# compiler would omit the assembly reference, and the trimmer did not even receive it as input.

Copy link
Member

Choose a reason for hiding this comment

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

Is this (still?) a problem? #8904 updated things to look for "transitive assembly references" (see also).

If we need to add explicit type references to work around missing transitive assembly references, maybe we need to revisit #8904?

@@ -48,6 +48,8 @@ public void ProcessAssembly (AssemblyDefinition assembly, string androidHttpClie
}

// Custom views in Android .xml files
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.

👍

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.

The RegisterTaskObjectAssemblyLocall call happens inside LoadCustomViewMapFile see

var cachedMap = engine?.GetRegisteredTaskObjectAssemblyLocal<Dictionary<string, HashSet<string>>> (mapFile, RegisteredTaskObjectLifetime.Build);

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When using TrimMode=link custom AndroidHttpClientHandlerType is not preserved
3 participants