From ce23e493c637485924dc6334a6af52b1fec55952 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 25 Jul 2022 15:58:21 -0500 Subject: [PATCH 1/3] [linker] preserve interfaces on Java types Fixes: https://github.com/xamarin/xamarin-android/issues/7097 Context: https://github.com/xamarin/monodroid/commit/a619cbea33e1b7cd8ffd0352bb37ac551d324f67 Usage of a `Google.Android.Material.TextField.TextInputEditText`: var filterBox = FindViewById(Resource.Id.filterBox); filterBox.TextChanged += (s, e) => { }; Crashes at runtime with: android.runtime.JavaProxyThrowable: System.TypeLoadException: Could not load type '{0}' from assembly '{1}'., Android.Text.ITextWatcherInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null at System.RuntimeTypeHandle.GetTypeByName(String , Boolean , Boolean , StackCrawlMark& , Boolean ) at System.RuntimeType.GetType(String , Boolean , Boolean , StackCrawlMark& ) at System.Type.GetType(String , Boolean ) at Android.Runtime.AndroidTypeManager.RegisterNativeMembers(JniType , Type , ReadOnlySpan`1 ) --- End of stack trace from previous location --- at Java.Interop.JniEnvironment.StaticMethods.CallStaticObjectMethod(JniObjectReference , JniMethodInfo , JniArgumentValue* ) at Android.Runtime.JNIEnv.FindClass(String ) at Android.Runtime.JNIEnv.AllocObject(String ) at Android.Runtime.JNIEnv.StartCreateInstance(String , String , JValue* ) at Android.Runtime.JNIEnv.StartCreateInstance(String , String , JValue[] ) at Android.Text.TextWatcherImplementor..ctor(Object , EventHandler`1 , EventHandler`1 , EventHandler`1 ) at Android.Widget.TextView.add_TextChanged(EventHandler`1 ) at AndroidApp1.MainActivity.OnCreate(Bundle savedInstanceState) at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_(IntPtr , IntPtr , IntPtr ) at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(_JniMarshal_PPL_V , IntPtr , IntPtr , IntPtr ) at crc64a6e0c00971f6cd91.MainActivity.n_onCreate(Native Method) at crc64a6e0c00971f6cd91.MainActivity.onCreate(MainActivity.java:29) The problem being that the linker completely removed the `ITextWatcher` interface, and the `Android.Text.TextWatcherImplementor` type no longer implemented that interface. It appears we should preserve interfaces in .NET 6 for Java types *except* if the type defines: [Android.Runtime.Register (DoNotGenerateAcw = true)] This is on types like `Android.App.Activity` where we would *not* need to preserve all the interfaces. However, types like `TextWatcherImplementor` would get their interfaces preserved appropriately. --- .../Linker/MonoDroid.Tuner/MarkJavaObjects.cs | 34 +++++++++++++++++++ .../LinkDescTest/MainActivityReplacement.cs | 3 ++ .../LinkDescTest/MaterialTextChanged.cs | 18 ++++++++++ .../Tests/InstallAndRunTests.cs | 18 ++++++++++ 4 files changed, 73 insertions(+) create mode 100644 tests/MSBuildDeviceIntegration/Resources/LinkDescTest/MaterialTextChanged.cs diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs index e8ac80cdef1..5d7ecdfab47 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs @@ -60,6 +60,9 @@ void PreserveJavaObjectImplementation (TypeDefinition type) PreserveIntPtrConstructor (type); PreserveAttributeSetConstructor (type); PreserveInvoker (type); +#if ILLINK + PreserveInterfaces (type); +#endif // ILLINK } void PreserveAttributeSetConstructor (TypeDefinition type) @@ -196,7 +199,38 @@ void PreserveInvoker (TypeDefinition type) PreserveConstructors (type, invoker); PreserveIntPtrConstructor (invoker); PreserveInterfaceMethods (type, invoker); +#if ILLINK + PreserveInterfaces (invoker); +#endif // ILLINK + } + +#if ILLINK + void PreserveInterfaces (TypeDefinition type) + { + if (!type.HasInterfaces) + return; + + // Return if [Register(DoNotGenerateAcw=true)] is on the type + if (type.HasCustomAttributes) { + foreach (var attr in type.CustomAttributes) { + if (attr.AttributeType.FullName == "Android.Runtime.RegisterAttribute") { + foreach (var property in attr.Properties) { + if (property.Name == "DoNotGenerateAcw") { + if (property.Argument.Value is bool value && value) + return; + break; + } + } + break; + } + } + } + + foreach (var iface in type.Interfaces) { + Annotations.Mark (iface.InterfaceType.Resolve ()); + } } +#endif // ILLINK TypeDefinition GetInvokerType (TypeDefinition type) { diff --git a/tests/MSBuildDeviceIntegration/Resources/LinkDescTest/MainActivityReplacement.cs b/tests/MSBuildDeviceIntegration/Resources/LinkDescTest/MainActivityReplacement.cs index a8f648afb2c..31aa2f0ef4e 100644 --- a/tests/MSBuildDeviceIntegration/Resources/LinkDescTest/MainActivityReplacement.cs +++ b/tests/MSBuildDeviceIntegration/Resources/LinkDescTest/MainActivityReplacement.cs @@ -115,6 +115,9 @@ protected override void OnCreate(Bundle bundle) // [Test] Post Android.Util.Log.Info(TAG, HttpClientTest.Post ()); + // [Test] TextChanged + Android.Util.Log.Info (TAG, MaterialTextChanged.TextChanged (this)); + var cldt = new CustomLinkerDescriptionTests(); Android.Util.Log.Info(TAG, cldt.TryAccessNonXmlPreservedMethodOfLinkerModeFullClass()); Android.Util.Log.Info(TAG, LinkTestLib.Bug21578.MulticastOption_ShouldNotBeStripped()); diff --git a/tests/MSBuildDeviceIntegration/Resources/LinkDescTest/MaterialTextChanged.cs b/tests/MSBuildDeviceIntegration/Resources/LinkDescTest/MaterialTextChanged.cs new file mode 100644 index 00000000000..721b01d7ba8 --- /dev/null +++ b/tests/MSBuildDeviceIntegration/Resources/LinkDescTest/MaterialTextChanged.cs @@ -0,0 +1,18 @@ +using System; +using Android.Content; +using Google.Android.Material.TextField; + +public class MaterialTextChanged +{ + // [Test] + public static string TextChanged (Context context) + { + try { + var view = new TextInputEditText (context); + view.TextChanged += (s, e) => { }; + return $"[PASS] {nameof (MaterialTextChanged)}.{nameof (TextChanged)}"; + } catch (Exception ex) { + return $"[FAIL] {nameof (MaterialTextChanged)}.{nameof (TextChanged)} FAILED: {ex}"; + } + } +} diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs index d00dc301db8..9fe8bc19948 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs @@ -421,6 +421,24 @@ public class LinkModeFullClass { new BuildItem ("ProjectReference", "..\\Library1\\Library1.csproj"), new BuildItem ("ProjectReference", "..\\LinkTestLib\\LinkTestLib.csproj"), }, + PackageReferences = { + KnownPackages.AndroidXMigration, + KnownPackages.AndroidXAppCompat, + KnownPackages.AndroidXAppCompatResources, + KnownPackages.AndroidXBrowser, + KnownPackages.AndroidXMediaRouter, + KnownPackages.AndroidXLegacySupportV4, + KnownPackages.AndroidXLifecycleLiveData, + KnownPackages.XamarinGoogleAndroidMaterial, + }, + Sources = { + new BuildItem.Source ("MaterialTextChanged.cs") { + TextContent = () => { + using (var sr = new StreamReader (typeof (InstallAndRunTests).Assembly.GetManifestResourceStream ("Xamarin.Android.Build.Tests.Resources.LinkDescTest.MaterialTextChanged.cs"))) + return sr.ReadToEnd (); + }, + }, + }, OtherBuildItems = { new BuildItem ("LinkDescription", "linker.xml") { TextContent = () => linkMode == AndroidLinkMode.SdkOnly ? "" : @" From b7d94a26e6b7642d843227ec78a36fff248552d5 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 8 Sep 2022 11:37:07 -0500 Subject: [PATCH 2/3] Add check for JniTypeSignature.GenerateJavaPeer==false --- .../Linker/MonoDroid.Tuner/MarkJavaObjects.cs | 40 ++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs index 5d7ecdfab47..d9256dcccb2 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs @@ -210,25 +210,47 @@ void PreserveInterfaces (TypeDefinition type) if (!type.HasInterfaces) return; - // Return if [Register(DoNotGenerateAcw=true)] is on the type - if (type.HasCustomAttributes) { - foreach (var attr in type.CustomAttributes) { - if (attr.AttributeType.FullName == "Android.Runtime.RegisterAttribute") { + if (!ShouldPreserveInterfaces (type)) + return; + + foreach (var iface in type.Interfaces) { + Annotations.Mark (iface.InterfaceType.Resolve ()); + } + } + + // False if [Register(DoNotGenerateAcw=true)] is on the type + // False if [JniTypeSignature(GenerateJavaPeer=false)] is on the type + bool ShouldPreserveInterfaces (TypeDefinition type) + { + if (!type.HasCustomAttributes) + return true; + + foreach (var attr in type.CustomAttributes) { + switch (attr.AttributeType.FullName) { + case "Android.Runtime.RegisterAttribute": foreach (var property in attr.Properties) { if (property.Name == "DoNotGenerateAcw") { if (property.Argument.Value is bool value && value) - return; + return false; break; } } break; - } + case "Java.Interop.JniTypeSignatureAttribute": + foreach (var property in attr.Properties) { + if (property.Name == "GenerateJavaPeer") { + if (property.Argument.Value is bool value && !value) + return false; + break; + } + } + break; + default: + break; } } - foreach (var iface in type.Interfaces) { - Annotations.Mark (iface.InterfaceType.Resolve ()); - } + return true; } #endif // ILLINK From 195cb9f18edf6ef8a271746b3ada36e996fec15f Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 9 Sep 2022 15:53:40 -0500 Subject: [PATCH 3/3] Only preserve IJavaPeerable interfaces --- .../Linker/MonoDroid.Tuner/MarkJavaObjects.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs index d9256dcccb2..66893206bd8 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs @@ -214,7 +214,10 @@ void PreserveInterfaces (TypeDefinition type) return; foreach (var iface in type.Interfaces) { - Annotations.Mark (iface.InterfaceType.Resolve ()); + var td = iface.InterfaceType.Resolve (); + if (!td.ImplementsIJavaPeerable ()) + continue; + Annotations.Mark (td); } }