[net10.0] Trimmable view handlers

[simonrozsival]

This PR supersedes #28115

Description of Change

This proof of concept explores how we could get rid of the long DI setup code in AppHostBuilderExtensions:

internal static IMauiHandlersCollection AddControlsHandlers(this IMauiHandlersCollection handlersCollection)
{
    handlersCollection.AddHandler<CollectionView, CollectionViewHandler>();
    handlersCollection.AddHandler<CarouselView, CarouselViewHandler>();
    handlersCollection.AddHandler<Application, ApplicationHandler>();
    handlersCollection.AddHandler<ActivityIndicator, ActivityIndicatorHandler>();
    handlersCollection.AddHandler<BoxView, BoxViewHandler>();
    // ...

Listing all the classes this way makes it impossible for the trimmer to remove any of them.

The idea in this PR is to connect the element and the element handler using an attribute instead of looking up the handler in the DI container:

[ElementHandler<LabelHandler>]
partial class Label
{
    static Label()
    {
        RemappingHelper.RemapIfNeeded(typeof(VisualElement), VisualElement.RemapIfNeeded);

        // ...
    }
}

EDIT: The IRemappable interface used in earlier iteration has been removed.

if the control isn't used anywhere in the app, the trimmer will remove the element type. When the type is removed, the handler type is removed as well, because the only link to the handler type is the custom class attribute.

The DI mechanism is still checked first before looking for the attribute for two reasons:

1. Backwards compatibility: if the developer registers their own handlers in their app/library through DI today, it should continue working. They will need to make changes to their code to make their controls trimmable as well.
2. Overwriting the defaults: It should be possible to change the default handlers for built in controls. I'm not sure how useful this is, but we should keep that possibility around.

Notes:

• I'm sharing this Draft to get early feedback. I'm still not 100% sure this is the right way to do it. I'm especially not confident in the way RemapForControls would be handled.
• This PR shows what the changes would look like for two controls: Label and CheckBox. These two are chosen because Label is in the dotnet new maui app and so we can test that the handler for that control is created correctly, and CheckBox is NOT in the sample app, so we can verify that the trimmer removes this class + the CheckBoxHandler when trim mode is set to full.

Feedback is very welcome!

Fixes

• Improve linking of Microsoft.Maui.Controls.* assemblies #8593

/cc @jonathanpeppers @PureWeen @mattleibow @ivanpovazan

[mattleibow]

This can be a static abstract method: https://github.com/dotnet/runtime/blob/82c916f423fdbb3bc7a2425a253f961363078f8b/src/libraries/System.Private.CoreLib/src/System/IParsable.cs#L20

For example, I did this horrible thing: https://sharplab.io/#v2:C4LgTgrgdgPgAgJgAwFgBQiCM65IARyYB0ASgKYBmANmQMbACWA9lANzroCiAthFQIbAyAVQCSACihkA7ngAy/AEZkq4gJRr2aHn0EiJU2QuWqNWnQKFjJM+UpXrNHNDgAseC3uvGVeASbV0AG90PDC8ADd+MDxBYBiAXj97KiIAcTJgABUATwAHMnV0zIBhCABnYCZuAEFgeIZFCCEAHk4abjIoYAAJfigAExowOoamoQA+dS1wyOi8AAt+obJE2PqwAEIiErAyPT7B4ennWcIATnElo9WnNABfZwB6J7wAWRqxPBKAeQA5LIkH5yADK3x+ABFOM4ANrkbj8PIFAbtMidbqHFZgFo+KiY4YTAC6OAQdhMwXQjxcaBe70+ojwPRqfwhck4JDwoj+4JI0OpiDJKnxqzwIE5uOFYAANJz4Yjkaj0b1lsMKWgzgBmAiYABsBHccry6lC4RC6tm4Qu4gARIbkXhgAsyIK8SrVkRrXdZlSqehaR8vr9eZy3gAFOTOBjdVYUfi0Z2iCVumJi0SKrrKm5gNW+tBRoRgWPxznpjHJnPPV5/TgAdR5nDwIKywgAYi3I9HC3GE3ayCiOhnJaKSwOy1m1WddbFFJUwHHgPq8IaTg9nEpZ/OCKTS5msaMwI1ms6xfvD0I1QBIOBa9fxTdp0e74bfPYHZMr3O3uf0LdLtHyvsd0lU9xmPDxH2AjYzzICtqXKfYaAGX8gOTECjxaLJJQmYcUKzNDz3NMJpCdPY8Ew5NhwfNFB2TGVDGNNAzU1PAmAiVYDwGBNcKxF99iESUGNmJiLTCOAAHY8EMMiBK9cIfWceD+EQ39e37aixz3KDQIwrDh1U7jhnwmDCLwYjVmdcis0o/SINoySZAY4TRK1Vj2IYTiR3Up8RV2PiyBkk0wici1DRKJThh05MJh2cKVxE0SJKkyysTiuTKWcQg9QFULwtWSKswmQLTJIiyh1TGyvMlCdLSnHKqGOQITOC2ZkuGUh/yNWSwipWYiuvbUsvcML6vUIJ5LQIA===

[mattleibow]

oops, i see i used static ctors which are random in AOT, so pretend I did something else in that bit of code.

[simonrozsival]

I tried to use static abstract methods, but I struggled to get that working with netstandard. I think I would have to completely remove the Remap methods in netstandard which seems weird. Do we have some precedent for that already?

[mattleibow]

Yes. Netstandard is not supported. We just use it to access type information in controls. And I am pretty sure that is not being used. Just for vs as well.

We are using static interface members and default interface members already.

[simonrozsival]

So I ran into the problem that the static abstract methods can only be called via constrained generic type. This means we would need to have two different attributes for handlers:

[RemappableElementHandler<CheckBoxHandler>]
class CheckBox;

[ElementHandler<ProgressBarHandler>]
class ProgressBar;

If we kept this as a simple interface with an instance method, we can use pattern matching in RemappingHelper to see if the handler implements IRemappable or not.

Alternatively, we could put the remapping logic into the static constructor of the handler. That would be likely more efficient, but we would have even less control over the order in which the remappings are applied. If I understand it correctly, the order of remapping matters and it is not desirable to remap a base type after the subclass because it could shadow the more specialized remappings.

We could maybe just do something like this:

class VisualElement
{
    static VisualElement() => RemapForControls();
    internal new static void RemapForControls()
    {
        // make sure we don't override any previously registered remappings of other derived classes
        if (!RemappingHelper.ShouldRemap(typeof(VisualElement))
            return;

        Element.RemapForControls();
        // ... do what VisualElement needs to do
}

class CheckBox
{
    static CheckBox() => RemapForControls();
    internal new static void RemapForControls()
    {
        if (!RemappingHelper.ShouldRemap(typeof(CheckBox))
            remap;

        VisualElement.RemapForControls(); // the base ctor will run on its own, but there is no guarantee that the base cctor runs BEFORE the derived class cctor. to achieve the right ordering, we need to call the base remapping ourselves
        // ... do what CheckBox needs to do
    }
}

class Label
{
    static Label() => RemapForControls();    
    internal new static void RemapForControls()
    {
        if (!RemappingHelper.ShouldRemap(typeof(Label))
            return;

        VisualElement.RemapForControls(); 
        // ... do the label remapping
    }
}

It would be less lazy than what is in this PR, but it wouldn't be any worse than today. It would still be trimmable and it would require one less interface.

(I will need to work on the thread-safety aspect of the code. I don't think the code with the early returns is actually correct.)

[simonrozsival]

I realized this doesn't work. It's the element that can be remapped, not the handler. I need to figure out how to address this without having to change the API drastically. The cctors approach might be the best after all.

[rmarinho]

/rebase

[mattleibow]

@jfversluis and @StephaneDelcroix and FYI and source generators and thoughts?

[StephaneDelcroix]

this looks like a red flag for trimming.

[simonrozsival]

if I remember correctly, this is necessary to use this extension method, which is trimming and AOT-friendly:

viewType.GetCustomAttribute<ElementHandlerAttribute>()