Enable DefaultValueAttributeSupport in partial trim mode.

[matouskozak]

Currently, users can experience runtime crashes related to the use of DefaultValue attributes (taking type and string) without any trim warnings (see dotnet/runtime#109724). This is because the trim warnings are disable in partial trim mode and this use-case of DefaultValue attributes is not trim compatible.

Workaround around this is to enable _DefaultValueAttributeSupport to prevent customer apps from crashing without trim warnings. App will still crash in TrimMode="full" but user would get warned during compile time.

[dellis1972]

Looks good. Is there any way to unit test this on one of our one device tests?

[jonathanpeppers]

Should we just add Sven's minimal repro:

• RuntimeInstanceNotAllowed Error Initializing XmlSerializer on Android Release Build in .NET 9 runtime#109724 (comment)

To a new test in a new System.Xml folder here:

• https://github.com/dotnet/android/tree/main/tests/Mono.Android-Tests

[matouskozak]

Should we just add Sven's minimal repro:

• RuntimeInstanceNotAllowed Error Initializing XmlSerializer on Android Release Build in .NET 9 runtime#109724 (comment)

To a new test in a new System.Xml folder here:

• https://github.com/dotnet/android/tree/main/tests/Mono.Android-Tests

Thanks, added in 14cfd74. What do you think?

[jonathanpeppers]

It looks like a projitems file needed an update, I pushed this change.

We used to share the files between Xamarin & .NET; it could be refactored in the future.

[matouskozak]

The failures are related:

System.InvalidOperationException : XmlTypeReflectionError, System.XmlTests.C
----> System.ArgumentException : RuntimeInstanceNotAllowed

at System.Xml.Serialization.XmlReflectionImporter.ImportTypeMapping(TypeModel , String , ImportContext , String , XmlAttributes , Boolean , Boolean , RecursionLimiter )
   at System.Xml.Serialization.XmlReflectionImporter.ImportTypeMapping(TypeModel , String , ImportContext , String , XmlAttributes , RecursionLimiter )
   at System.Xml.Serialization.XmlReflectionImporter.ImportElement(TypeModel , XmlRootAttribute , String , RecursionLimiter )
   at System.Xml.Serialization.XmlReflectionImporter.ImportTypeMapping(Type , XmlRootAttribute , String )
   at System.Xml.Serialization.XmlSerializer..ctor(Type type, String defaultNamespace)
   at System.Xml.Serialization.XmlSerializer..ctor(Type type)
   at System.XmlTests.XmlSerializerTest.TrimmingDefaultValueAttribute()
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags )
--ArgumentException
   at System.ComponentModel.DefaultValueAttribute.get_Value()
   at System.Xml.Serialization.XmlAttributes..ctor(ICustomAttributeProvider )
   at System.Xml.Serialization.XmlReflectionImporter.GetAttributes(MemberInfo )
   at System.Xml.Serialization.XmlReflectionImporter.InitializeStructMembers(StructMapping , StructModel , Boolean , String , RecursionLimiter )
   at System.Xml.Serialization.XmlReflectionImporter.ImportStructLikeMapping(StructModel , String , Boolean , XmlAttributes , RecursionLimiter )
   at System.Xml.Serialization.XmlReflectionImporter.ImportTypeMapping(TypeModel , String , ImportContext , String , XmlAttributes , Boolean , Boolean , RecursionLimiter )

[jonathanpeppers]

It passed in Debug mode, which was broken before?

This app has TrimMode=full:

android/tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Mono.Android.NET-Tests.csproj

Lines 37 to 39 in a1fab8b

	<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
	<AndroidLinkTool Condition=" '$(AndroidLinkTool)' == '' ">r8</AndroidLinkTool>
	<TrimMode Condition=" '$(TrimMode)' == '' ">full</TrimMode>

Can the test check a feature flag at runtime (AppContext.TryGetSwitch), and ignore in some cases? Ideally we'd get a green test in Debug and the rest ignore.

[matouskozak]

It passed in Debug mode, which was broken before?

This app has TrimMode=full:

android/tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Mono.Android.NET-Tests.csproj

Lines 37 to 39 in a1fab8b

	<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
	<AndroidLinkTool Condition=" '$(AndroidLinkTool)' == '' ">r8</AndroidLinkTool>
	<TrimMode Condition=" '$(TrimMode)' == '' ">full</TrimMode>

Can the test check a feature flag at runtime (AppContext.TryGetSwitch), and ignore in some cases? Ideally we'd get a green test in Debug and the rest ignore.

No, it was the release (related to illink) that was broken but the "fix" is only for TrimMode="partial" so if the test are running in full trim mode then they will fail. We can either extend the "fix" to all trim modes or just add the _DefaultValueAttributeSupportto

android/tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/Mono.Android.NET-Tests.csproj

Lines 40 to 41 in a1fab8b

	<!-- Trimmer switches required for tests -->
	<JsonSerializerIsReflectionEnabledByDefault>true</JsonSerializerIsReflectionEnabledByDefault>

Note: I limited the "fix" to partial mode because that's where no trim warnings are shown and yet it failed. In full mode the warnings will show potential issues so that users can add approriate feature switches if needed.

[jonathanpeppers]

Let me add an extra test run, that runs the tests with TrimMode=partial.

[jonathanpeppers]

I think it's working now:

I reran, because there are some other random, http failures.