From d521ac0280c0ad165570077a860cb1846025010b Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Tue, 13 Sep 2022 17:38:59 +0100 Subject: [PATCH] [Xamarin.Android.Build.Tasks] AndroidLinkResources and Styleables (#7306) Fixes: https://github.com/xamarin/xamarin-android/issues/7194 Context: https://github.com/dotnet/maui/pull/7038 The initial version of `$(AndroidLinkResources)` (9e6ce03c) was too broad in its removal of Resource classes and fields. Certain fields such as `Styleable` arrays were not called using the IL `stsfld` opcode. As a result they could not be easily replaced with constant usage. However, the linker removed *all* the fields from the `Resource` nested types. This would result in the following error at runtime: System.BadImageFormatException: 'Could not resolve field token 0x0400000b' This was because the `int[]` fields were removed as part of the linking process. Fix this by leaving the `int[]` fields in the `Resource` nested types instead of removing them. We can still remove all the other `int` fields. We now also need to fix up the `Resource` nested type constructors to replace the `int` field access with the constant values like we do for the rest of the app. This was not required previously because these constructors were removed, but now we have to keep them because the static array initialization takes place in these constructors. --- .../HelloLibrary/HelloLibrary.csproj | 3 + .../HelloLibrary/LibraryActivity.cs | 2 +- .../HelloLibrary/Resources/values/Attr.xml | 7 ++ .../HelloWorld/HelloWorld/HelloWorld.csproj | 1 + .../HelloWorld/Resources/values/Attr.xml | 7 ++ .../MonoDroid.Tuner/LinkDesignerBase.cs | 56 ++++++++- .../RemoveResourceDesignerStep.cs | 7 +- .../Tests/InstallAndRunTests.cs | 111 ++++++++++++++++++ 8 files changed, 190 insertions(+), 4 deletions(-) create mode 100644 samples/HelloWorld/HelloLibrary/Resources/values/Attr.xml create mode 100644 samples/HelloWorld/HelloWorld/Resources/values/Attr.xml diff --git a/samples/HelloWorld/HelloLibrary/HelloLibrary.csproj b/samples/HelloWorld/HelloLibrary/HelloLibrary.csproj index aa9cb8a9b8b..b613bde6304 100644 --- a/samples/HelloWorld/HelloLibrary/HelloLibrary.csproj +++ b/samples/HelloWorld/HelloLibrary/HelloLibrary.csproj @@ -14,6 +14,8 @@ 512 false portable + True + Resource + diff --git a/samples/HelloWorld/HelloLibrary/LibraryActivity.cs b/samples/HelloWorld/HelloLibrary/LibraryActivity.cs index f876c69d0f5..5e3147b8db6 100644 --- a/samples/HelloWorld/HelloLibrary/LibraryActivity.cs +++ b/samples/HelloWorld/HelloLibrary/LibraryActivity.cs @@ -16,7 +16,7 @@ using Android.Views; using Android.Widget; -namespace Mono.Samples.Hello +namespace HelloLibrary { [Activity(Label = "Library Activity", Name="mono.samples.hello.LibraryActivity")] public class LibraryActivity : Activity diff --git a/samples/HelloWorld/HelloLibrary/Resources/values/Attr.xml b/samples/HelloWorld/HelloLibrary/Resources/values/Attr.xml new file mode 100644 index 00000000000..6b39720b438 --- /dev/null +++ b/samples/HelloWorld/HelloLibrary/Resources/values/Attr.xml @@ -0,0 +1,7 @@ + + + + + + + \ No newline at end of file diff --git a/samples/HelloWorld/HelloWorld/HelloWorld.csproj b/samples/HelloWorld/HelloWorld/HelloWorld.csproj index c0313fa8c99..17f2e7ab90d 100644 --- a/samples/HelloWorld/HelloWorld/HelloWorld.csproj +++ b/samples/HelloWorld/HelloWorld/HelloWorld.csproj @@ -66,6 +66,7 @@ + diff --git a/samples/HelloWorld/HelloWorld/Resources/values/Attr.xml b/samples/HelloWorld/HelloWorld/Resources/values/Attr.xml new file mode 100644 index 00000000000..5fbb641e067 --- /dev/null +++ b/samples/HelloWorld/HelloWorld/Resources/values/Attr.xml @@ -0,0 +1,7 @@ + + + + + + + \ No newline at end of file diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkDesignerBase.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkDesignerBase.cs index 947ca275d78..928fdfd6e4f 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkDesignerBase.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkDesignerBase.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using Mono.Cecil.Cil; using System.Text.RegularExpressions; +using Mono.Collections.Generic; #if ILLINK using Microsoft.Android.Sdk.ILLink; #endif @@ -69,8 +70,17 @@ protected bool FindResourceDesigner (AssemblyDefinition assembly, bool mainAppli protected void ClearDesignerClass (TypeDefinition designer) { LogMessage ($" TryRemoving {designer.FullName}"); - designer.NestedTypes.Clear (); - designer.Methods.Clear (); + // for each of the nested types clear all but the + // int[] fields. + for (int i = designer.NestedTypes.Count -1; i >= 0; i--) { + var nestedType = designer.NestedTypes [i]; + RemoveFieldsFromType (nestedType, designer.Module); + if (nestedType.Fields.Count == 0) { + // no fields we do not need this class at all. + designer.NestedTypes.RemoveAt (i); + } + } + RemoveUpdateIdValues (designer); designer.Fields.Clear (); designer.Properties.Clear (); designer.CustomAttributes.Clear (); @@ -117,6 +127,48 @@ protected void FixType (TypeDefinition type, TypeDefinition localDesigner) } } + protected void RemoveFieldsFromType (TypeDefinition type, ModuleDefinition module) + { + for (int i = type.Fields.Count - 1; i >= 0; i--) { + var field = type.Fields [i]; + if (field.FieldType.IsArray) { + continue; + } + LogMessage ($"Removing {type.Name}::{field.Name}"); + type.Fields.RemoveAt (i); + } + } + + protected void RemoveUpdateIdValues (TypeDefinition type) + { + foreach (var method in type.Methods) { + if (method.Name.Contains ("UpdateIdValues")) { + FixUpdateIdValuesBody (method); + } else { + FixBody (method.Body, type); + } + } + + foreach (var nestedType in type.NestedTypes) { + RemoveUpdateIdValues (nestedType); + } + } + + protected void FixUpdateIdValuesBody (MethodDefinition method) + { + List finalInstructions = new List (); + Collection instructions = method.Body.Instructions; + for (int i = 0; i < method.Body.Instructions.Count-1; i++) { + Instruction instruction = instructions[i]; + string line = instruction.ToString (); + bool found = line.Contains ("Int32[]") || instruction.OpCode == OpCodes.Ret; + if (!found) { + method.Body.Instructions.Remove (instruction); + i--; + } + } + } + protected void FixupAssemblyTypes (AssemblyDefinition assembly, TypeDefinition designer) { foreach (ModuleDefinition module in assembly.Modules) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs index db75512db24..2ef7e074cdc 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveResourceDesignerStep.cs @@ -55,14 +55,19 @@ protected override void FixBody (MethodBody body, TypeDefinition designer) Dictionary instructions = new Dictionary(); var processor = body.GetILProcessor (); string designerFullName = $"{designer.FullName}/"; + bool isDesignerMethod = designerFullName.Contains (body.Method.DeclaringType.FullName); + string declaringTypeName = body.Method.DeclaringType.Name; foreach (var i in body.Instructions) { string line = i.ToString (); - if (line.Contains (designerFullName) && !instructions.ContainsKey (i)) + if ((line.Contains (designerFullName) || (isDesignerMethod && i.OpCode == OpCodes.Stsfld)) && !instructions.ContainsKey (i)) { var match = opCodeRegex.Match (line); if (match.Success && match.Groups.Count == 5) { string key = match.Groups[4].Value.Replace (designerFullName, string.Empty); + if (isDesignerMethod) { + key = declaringTypeName +"::" + key; + } if (designerConstants.ContainsKey (key) && !instructions.ContainsKey (i)) instructions.Add(i, designerConstants [key]); } diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs index d00dc301db8..90a922b03f4 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs @@ -712,5 +712,116 @@ public void SingleProject_ApplicationId () Path.Combine (Root, builder.ProjectDirectory, "startup-logcat.log")); Assert.IsTrue (didStart, "Activity should have started."); } + + [Test] + public void AppWithStyleableUsageRuns ([Values (true, false)] bool isRelease, [Values (true, false)] bool linkResources) + { + AssertHasDevices (); + + var rootPath = Path.Combine (Root, "temp", TestName); + var lib = new XamarinAndroidLibraryProject () { + ProjectName = "Styleable.Library" + }; + + lib.AndroidResources.Add (new AndroidItem.AndroidResource ("Resources\\values\\styleables.xml") { + TextContent = () => @" + + + + + +", + }); + lib.AndroidResources.Add (new AndroidItem.AndroidResource ("Resources\\layout\\librarylayout.xml") { + TextContent = () => @" + +", + }); + lib.Sources.Add (new BuildItem.Source ("MyLibraryLayout.cs") { + TextContent = () => @"using System; + +namespace Styleable.Library { + public class MyLibraryLayout : Android.Widget.LinearLayout + { + + public MyLibraryLayout (Android.Content.Context context, Android.Util.IAttributeSet attrs) : base (context, attrs) + { + Android.Content.Res.TypedArray a = context.Theme.ObtainStyledAttributes (attrs, Resource.Styleable.MyLibraryView, 0,0); + try { + bool b = a.GetBoolean (Resource.Styleable.MyLibraryView_MyBool, defValue: false); + if (!b) + throw new Exception (""MyBool was not true.""); + int i = a.GetInteger (Resource.Styleable.MyLibraryView_MyInt, defValue: -1); + if (i != 128) + throw new Exception (""MyInt was not 128.""); + } + finally { + a.Recycle(); + } + } + } +}" + }); + + proj = new XamarinAndroidApplicationProject () { + IsRelease = isRelease, + }; + proj.AddReference (lib); + + proj.AndroidResources.Add (new AndroidItem.AndroidResource ("Resources\\values\\styleables.xml") { + TextContent = () => @" + + + + + +", + }); + proj.SetProperty ("AndroidLinkResources", linkResources ? "False" : "True"); + proj.LayoutMain = proj.LayoutMain.Replace ("", ""); + + proj.MainActivity = proj.DefaultMainActivity.Replace ("//${AFTER_MAINACTIVITY}", +@"public class MyLayout : Android.Widget.LinearLayout +{ + + public MyLayout (Android.Content.Context context, Android.Util.IAttributeSet attrs) : base (context, attrs) + { + Android.Content.Res.TypedArray a = context.Theme.ObtainStyledAttributes (attrs, Resource.Styleable.MyView, 0,0); + try { + bool b = a.GetBoolean (Resource.Styleable.MyView_MyBool, defValue: false); + if (!b) + throw new Exception (""MyBool was not true.""); + int i = a.GetInteger (Resource.Styleable.MyView_MyInt, defValue: -1); + if (i != 128) + throw new Exception (""MyInt was not 128.""); + } + finally { + a.Recycle(); + } + } +} +"); + + var abis = new string [] { "armeabi-v7a", "arm64-v8a", "x86", "x86_64" }; + proj.SetAndroidSupportedAbis (abis); + var libBuilder = CreateDllBuilder (Path.Combine (rootPath, lib.ProjectName)); + Assert.IsTrue (libBuilder.Build (lib), "Library should have built succeeded."); + builder = CreateApkBuilder (Path.Combine (rootPath, proj.ProjectName)); + + + Assert.IsTrue (builder.Install (proj), "Install should have succeeded."); + + if (Builder.UseDotNet) + Assert.True (builder.RunTarget (proj, "Run"), "Project should have run."); + else if (CommercialBuildAvailable) + Assert.True (builder.RunTarget (proj, "_Run"), "Project should have run."); + else + AdbStartActivity ($"{proj.PackageName}/{proj.JavaPackageName}.MainActivity"); + + var didStart = WaitForActivityToStart (proj.PackageName, "MainActivity", + Path.Combine (Root, builder.ProjectDirectory, "startup-logcat.log")); + Assert.IsTrue (didStart, "Activity should have started."); + } } }