Skip to content

Commit fa20c91

Browse files
simonrozsivalCopilot
simonrozsival
and
Copilot
authored
[TrimmableTypeMap] Fix manifest builder gaps: SupportsGLTexture, UsesPermissionFlags, RoundIcon, dedup (#11044)
## Summary Fix gaps in the assembly-level manifest builder compared to the old `ManifestDocument` path. ## Changes ### Scanner - **SupportsGLTexture**: Add `SupportsGLTextureInfo` record and `SupportsGLTextureAttribute` handling in `ScanAssemblyAttributes` - **UsesPermissionFlags**: Add `UsesPermissionFlags` property to `UsesPermissionInfo`, extract in scanner - **Perf**: Guard `ParseNameAndProperties` with `KnownAssemblyAttributes` HashSet to skip non-manifest attributes early ### Generator - **SupportsGLTexture**: Emit `<supports-gl-texture>` element in `AssemblyLevelElementBuilder` - **UsesPermissionFlags**: Emit `android:usesPermissionFlags` attribute on `<uses-permission>` - **RoundIcon**: Add `android:roundIcon` mapping for `<permission>`, `<permission-group>`, and `<permission-tree>` elements - **BackupAgent/ManageSpaceActivity**: Resolve managed type name strings to JNI names via scanned peers and emit `android:backupAgent` / `android:manageSpaceActivity` on `<application>` - **Dedup**: Deduplicate `<permission-group>` and `<permission-tree>` against manifest template (matching existing `<permission>` and `<uses-permission>` dedup) ### Cleanup - Remove redundant `#nullable enable` from `AssemblyLevelElementBuilder.cs` - Dispose `JavaPeerScanner` in `FixtureTestBase` after scan completes - Remove unused parameters from `ToComponentInfo` - Use `HashSet.Add()` pattern for all dedup sets to prevent cross-assembly duplicates ## Tests - Scanner tests: `SupportsGLTexture_ConstructorArg`, `UsesPermission_Flags` - Generator tests: `AssemblyLevel_SupportsGLTexture`, `AssemblyLevel_UsesPermissionFlags`, `AssemblyLevel_PermissionRoundIcon` Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1275ade commit fa20c91

12 files changed

Lines changed: 226 additions & 14 deletions

File tree

‎src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/AssemblyLevelElementBuilder.cs‎

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
#nullable enable
2-
31
using System;
42
using System.Collections.Generic;
53
using System.Globalization;
@@ -21,63 +19,73 @@ internal static void AddAssemblyLevelElements (XElement manifest, XElement app,
2119
{
2220
var existingPermissions = new HashSet<string> (
2321
manifest.Elements ("permission").Select (e => (string?)e.Attribute (AttName)).OfType<string> ());
22+
var existingPermissionGroups = new HashSet<string> (
23+
manifest.Elements ("permission-group").Select (e => (string?)e.Attribute (AttName)).OfType<string> ());
24+
var existingPermissionTrees = new HashSet<string> (
25+
manifest.Elements ("permission-tree").Select (e => (string?)e.Attribute (AttName)).OfType<string> ());
2426
var existingUsesPermissions = new HashSet<string> (
2527
manifest.Elements ("uses-permission").Select (e => (string?)e.Attribute (AttName)).OfType<string> ());
2628

2729
// <permission> elements
2830
foreach (var perm in info.Permissions) {
29-
if (string.IsNullOrEmpty (perm.Name) || existingPermissions.Contains (perm.Name)) {
31+
if (string.IsNullOrEmpty (perm.Name) || !existingPermissions.Add (perm.Name)) {
3032
continue;
3133
}
3234
var element = new XElement ("permission", new XAttribute (AttName, perm.Name));
3335
PropertyMapper.MapDictionaryProperties (element, perm.Properties, "Label", "label");
3436
PropertyMapper.MapDictionaryProperties (element, perm.Properties, "Description", "description");
3537
PropertyMapper.MapDictionaryProperties (element, perm.Properties, "Icon", "icon");
38+
PropertyMapper.MapDictionaryProperties (element, perm.Properties, "RoundIcon", "roundIcon");
3639
PropertyMapper.MapDictionaryProperties (element, perm.Properties, "PermissionGroup", "permissionGroup");
3740
PropertyMapper.MapDictionaryEnumProperty (element, perm.Properties, "ProtectionLevel", "protectionLevel", AndroidEnumConverter.ProtectionToString);
3841
manifest.Add (element);
3942
}
4043

4144
// <permission-group> elements
4245
foreach (var pg in info.PermissionGroups) {
43-
if (string.IsNullOrEmpty (pg.Name)) {
46+
if (string.IsNullOrEmpty (pg.Name) || !existingPermissionGroups.Add (pg.Name)) {
4447
continue;
4548
}
4649
var element = new XElement ("permission-group", new XAttribute (AttName, pg.Name));
4750
PropertyMapper.MapDictionaryProperties (element, pg.Properties, "Label", "label");
4851
PropertyMapper.MapDictionaryProperties (element, pg.Properties, "Description", "description");
4952
PropertyMapper.MapDictionaryProperties (element, pg.Properties, "Icon", "icon");
53+
PropertyMapper.MapDictionaryProperties (element, pg.Properties, "RoundIcon", "roundIcon");
5054
manifest.Add (element);
5155
}
5256

5357
// <permission-tree> elements
5458
foreach (var pt in info.PermissionTrees) {
55-
if (string.IsNullOrEmpty (pt.Name)) {
59+
if (string.IsNullOrEmpty (pt.Name) || !existingPermissionTrees.Add (pt.Name)) {
5660
continue;
5761
}
5862
var element = new XElement ("permission-tree", new XAttribute (AttName, pt.Name));
5963
PropertyMapper.MapDictionaryProperties (element, pt.Properties, "Label", "label");
6064
PropertyMapper.MapDictionaryProperties (element, pt.Properties, "Icon", "icon");
65+
PropertyMapper.MapDictionaryProperties (element, pt.Properties, "RoundIcon", "roundIcon");
6166
manifest.Add (element);
6267
}
6368

6469
// <uses-permission> elements
6570
foreach (var up in info.UsesPermissions) {
66-
if (string.IsNullOrEmpty (up.Name) || existingUsesPermissions.Contains (up.Name)) {
71+
if (string.IsNullOrEmpty (up.Name) || !existingUsesPermissions.Add (up.Name)) {
6772
continue;
6873
}
6974
var element = new XElement ("uses-permission", new XAttribute (AttName, up.Name));
7075
if (up.MaxSdkVersion.HasValue) {
7176
element.SetAttributeValue (AndroidNs + "maxSdkVersion", up.MaxSdkVersion.Value.ToString (CultureInfo.InvariantCulture));
7277
}
78+
if (!string.IsNullOrEmpty (up.UsesPermissionFlags)) {
79+
element.SetAttributeValue (AndroidNs + "usesPermissionFlags", up.UsesPermissionFlags);
80+
}
7381
manifest.Add (element);
7482
}
7583

7684
// <uses-feature> elements
7785
var existingFeatures = new HashSet<string> (
7886
manifest.Elements ("uses-feature").Select (e => (string?)e.Attribute (AttName)).OfType<string> ());
7987
foreach (var uf in info.UsesFeatures) {
80-
if (uf.Name is not null && !existingFeatures.Contains (uf.Name)) {
88+
if (uf.Name is not null && existingFeatures.Add (uf.Name)) {
8189
var element = new XElement ("uses-feature",
8290
new XAttribute (AttName, uf.Name),
8391
new XAttribute (AndroidNs + "required", uf.Required ? "true" : "false"));
@@ -153,11 +161,59 @@ internal static void AddAssemblyLevelElements (XElement manifest, XElement app,
153161
}
154162
manifest.Add (element);
155163
}
164+
165+
// <supports-gl-texture> elements
166+
var existingGLTextures = new HashSet<string> (
167+
manifest.Elements ("supports-gl-texture").Select (e => (string?)e.Attribute (AttName)).OfType<string> ());
168+
foreach (var gl in info.SupportsGLTextures) {
169+
if (existingGLTextures.Add (gl.Name)) {
170+
manifest.Add (new XElement ("supports-gl-texture", new XAttribute (AttName, gl.Name)));
171+
}
172+
}
156173
}
157174

158-
internal static void ApplyApplicationProperties (XElement app, Dictionary<string, object?> properties)
175+
internal static void ApplyApplicationProperties (
176+
XElement app,
177+
Dictionary<string, object?> properties,
178+
IReadOnlyList<JavaPeerInfo> allPeers,
179+
Action<string>? warn = null)
159180
{
160181
PropertyMapper.ApplyMappings (app, properties, PropertyMapper.ApplicationPropertyMappings, skipExisting: true);
182+
183+
// BackupAgent and ManageSpaceActivity are Type properties — resolve managed type names to JNI names
184+
ApplyTypeProperty (app, properties, allPeers, "BackupAgent", "backupAgent", warn);
185+
ApplyTypeProperty (app, properties, allPeers, "ManageSpaceActivity", "manageSpaceActivity", warn);
186+
}
187+
188+
static void ApplyTypeProperty (
189+
XElement app,
190+
Dictionary<string, object?> properties,
191+
IReadOnlyList<JavaPeerInfo> allPeers,
192+
string propertyName,
193+
string xmlAttrName,
194+
Action<string>? warn)
195+
{
196+
if (app.Attribute (AndroidNs + xmlAttrName) is not null) {
197+
return;
198+
}
199+
if (!properties.TryGetValue (propertyName, out var value) || value is not string managedName || managedName.Length == 0) {
200+
return;
201+
}
202+
203+
// Strip assembly qualification if present (e.g., "MyApp.MyAgent, MyAssembly")
204+
var commaIndex = managedName.IndexOf (',');
205+
if (commaIndex > 0) {
206+
managedName = managedName.Substring (0, commaIndex).Trim ();
207+
}
208+
209+
foreach (var peer in allPeers) {
210+
if (peer.ManagedTypeName == managedName) {
211+
app.SetAttributeValue (AndroidNs + xmlAttrName, peer.JavaName.Replace ('/', '.'));
212+
return;
213+
}
214+
}
215+
216+
warn?.Invoke ($"Could not resolve {propertyName} type '{managedName}' to a Java peer for android:{xmlAttrName}.");
161217
}
162218

163219
internal static void AddInternetPermission (XElement manifest)

‎src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestGenerator.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class ManifestGenerator
3333
public bool ForceExtractNativeLibs { get; set; }
3434
public string? ManifestPlaceholders { get; set; }
3535
public string? ApplicationJavaClass { get; set; }
36+
public Action<string>? Warn { get; set; }
3637

3738
/// <summary>
3839
/// Generates the merged manifest from an optional pre-loaded template and writes it to <paramref name="outputPath"/>.
@@ -55,7 +56,7 @@ public IList<string> Generate (
5556

5657
// Apply assembly-level [Application] properties
5758
if (assemblyInfo.ApplicationProperties is not null) {
58-
AssemblyLevelElementBuilder.ApplyApplicationProperties (app, assemblyInfo.ApplicationProperties);
59+
AssemblyLevelElementBuilder.ApplyApplicationProperties (app, assemblyInfo.ApplicationProperties, allPeers, Warn);
5960
}
6061

6162
var existingTypes = new HashSet<string> (

‎src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyIndex.cs‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,13 +343,27 @@ static bool TryGetNamedArgument<T> (CustomAttributeValue<string> value, string a
343343
/// <summary>
344344
/// Scans assembly-level custom attributes for manifest-related data.
345345
/// </summary>
346+
static readonly HashSet<string> KnownAssemblyAttributes = new (StringComparer.Ordinal) {
347+
"PermissionAttribute",
348+
"PermissionGroupAttribute",
349+
"PermissionTreeAttribute",
350+
"UsesPermissionAttribute",
351+
"UsesFeatureAttribute",
352+
"UsesLibraryAttribute",
353+
"UsesConfigurationAttribute",
354+
"MetaDataAttribute",
355+
"PropertyAttribute",
356+
"SupportsGLTextureAttribute",
357+
"ApplicationAttribute",
358+
};
359+
346360
internal void ScanAssemblyAttributes (AssemblyManifestInfo info)
347361
{
348362
var asmDef = Reader.GetAssemblyDefinition ();
349363
foreach (var caHandle in asmDef.GetCustomAttributes ()) {
350364
var ca = Reader.GetCustomAttribute (caHandle);
351365
var attrName = GetCustomAttributeName (ca, Reader);
352-
if (attrName is null) {
366+
if (attrName is null || !KnownAssemblyAttributes.Contains (attrName)) {
353367
continue;
354368
}
355369

@@ -383,6 +397,11 @@ internal void ScanAssemblyAttributes (AssemblyManifestInfo info)
383397
case "PropertyAttribute":
384398
info.Properties.Add (CreatePropertyInfo (name, props));
385399
break;
400+
case "SupportsGLTextureAttribute":
401+
if (name.Length > 0) {
402+
info.SupportsGLTextures.Add (new SupportsGLTextureInfo { Name = name });
403+
}
404+
break;
386405
case "ApplicationAttribute":
387406
info.ApplicationProperties ??= new Dictionary<string, object?> (StringComparer.Ordinal);
388407
foreach (var kvp in props) {
@@ -421,7 +440,8 @@ internal void ScanAssemblyAttributes (AssemblyManifestInfo info)
421440
static UsesPermissionInfo CreateUsesPermissionInfo (string name, Dictionary<string, object?> props)
422441
{
423442
int? maxSdk = props.TryGetValue ("MaxSdkVersion", out var v) && v is int max ? max : null;
424-
return new UsesPermissionInfo { Name = name, MaxSdkVersion = maxSdk };
443+
string? flags = props.TryGetValue ("UsesPermissionFlags", out var f) && f is string s ? s : null;
444+
return new UsesPermissionInfo { Name = name, MaxSdkVersion = maxSdk, UsesPermissionFlags = flags };
425445
}
426446

427447
static UsesFeatureInfo CreateUsesFeatureInfo (string name, Dictionary<string, object?> props)

‎src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyManifestInfo.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ internal sealed class AssemblyManifestInfo
1313
public List<UsesConfigurationInfo> UsesConfigurations { get; } = [];
1414
public List<MetaDataInfo> MetaData { get; } = [];
1515
public List<PropertyInfo> Properties { get; } = [];
16+
public List<SupportsGLTextureInfo> SupportsGLTextures { get; } = [];
1617

1718
public Dictionary<string, object?>? ApplicationProperties { get; set; }
1819
}

‎src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ void ScanAssembly (AssemblyIndex index, Dictionary<string, JavaPeerInfo> results
246246
ActivationCtor = activationCtor,
247247
InvokerTypeName = invokerTypeName,
248248
IsGenericDefinition = isGenericDefinition,
249-
ComponentAttribute = ToComponentInfo (attrInfo, typeDef, index),
249+
ComponentAttribute = ToComponentInfo (attrInfo),
250250
};
251251

252252
results [fullName] = peer;
@@ -1560,7 +1560,7 @@ void CollectExportField (MethodDefinition methodDef, AssemblyIndex index, List<J
15601560
}
15611561
}
15621562

1563-
static ComponentInfo? ToComponentInfo (TypeAttributeInfo? attrInfo, TypeDefinition typeDef, AssemblyIndex index)
1563+
static ComponentInfo? ToComponentInfo (TypeAttributeInfo? attrInfo)
15641564
{
15651565
if (attrInfo is null) {
15661566
return null;

‎src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/SupportsGLTextureInfo.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
namespace Microsoft.Android.Sdk.TrimmableTypeMap;
2+
3+
internal sealed record SupportsGLTextureInfo
4+
{
5+
public required string Name { get; init; }
6+
}

‎src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesPermissionInfo.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ internal sealed record UsesPermissionInfo
44
{
55
public required string Name { get; init; }
66
public int? MaxSdkVersion { get; init; }
7+
public string? UsesPermissionFlags { get; init; }
78
}

‎tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/FixtureTestBase.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ static string TestFixtureAssemblyPath {
2323
}
2424

2525
static readonly Lazy<(List<JavaPeerInfo> peers, AssemblyManifestInfo manifestInfo)> _cachedScanResult = new (() => {
26-
var scanner = new JavaPeerScanner ();
26+
using var scanner = new JavaPeerScanner ();
2727
var peReader = new PEReader (File.OpenRead (TestFixtureAssemblyPath));
2828
var mdReader = peReader.GetMetadataReader ();
2929
var assemblyName = mdReader.GetString (mdReader.GetAssemblyDefinition ().Name);

‎tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/ManifestGeneratorTests.cs‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,4 +616,103 @@ public void ConfigChanges_EnumConversion ()
616616
Assert.True (parts.Contains ("screenSize"), "configChanges should contain 'screenSize'");
617617
Assert.Equal (3, parts.Length);
618618
}
619+
620+
[Fact]
621+
public void AssemblyLevel_SupportsGLTexture ()
622+
{
623+
var gen = CreateDefaultGenerator ();
624+
var info = new AssemblyManifestInfo ();
625+
info.SupportsGLTextures.Add (new SupportsGLTextureInfo { Name = "GL_OES_compressed_ETC1_RGB8_texture" });
626+
627+
var doc = GenerateAndLoad (gen, assemblyInfo: info);
628+
var element = doc.Root?.Elements ("supports-gl-texture")
629+
.FirstOrDefault (e => (string?)e.Attribute (AttName) == "GL_OES_compressed_ETC1_RGB8_texture");
630+
Assert.NotNull (element);
631+
}
632+
633+
[Fact]
634+
public void AssemblyLevel_UsesPermissionFlags ()
635+
{
636+
var gen = CreateDefaultGenerator ();
637+
var info = new AssemblyManifestInfo ();
638+
info.UsesPermissions.Add (new UsesPermissionInfo {
639+
Name = "android.permission.POST_NOTIFICATIONS",
640+
UsesPermissionFlags = "neverForLocation",
641+
});
642+
643+
var doc = GenerateAndLoad (gen, assemblyInfo: info);
644+
var perm = doc.Root?.Elements ("uses-permission")
645+
.FirstOrDefault (e => (string?)e.Attribute (AttName) == "android.permission.POST_NOTIFICATIONS");
646+
Assert.NotNull (perm);
647+
Assert.Equal ("neverForLocation", (string?)perm?.Attribute (AndroidNs + "usesPermissionFlags"));
648+
}
649+
650+
[Fact]
651+
public void AssemblyLevel_PermissionRoundIcon ()
652+
{
653+
var gen = CreateDefaultGenerator ();
654+
var info = new AssemblyManifestInfo ();
655+
info.Permissions.Add (new PermissionInfo {
656+
Name = "com.example.MY_PERMISSION",
657+
Properties = new Dictionary<string, object?> {
658+
["RoundIcon"] = "@mipmap/ic_launcher_round",
659+
},
660+
});
661+
662+
var doc = GenerateAndLoad (gen, assemblyInfo: info);
663+
var perm = doc.Root?.Elements ("permission")
664+
.FirstOrDefault (e => (string?)e.Attribute (AttName) == "com.example.MY_PERMISSION");
665+
Assert.NotNull (perm);
666+
Assert.Equal ("@mipmap/ic_launcher_round", (string?)perm?.Attribute (AndroidNs + "roundIcon"));
667+
}
668+
669+
[Fact]
670+
public void AssemblyLevel_ApplicationBackupAgent ()
671+
{
672+
var gen = CreateDefaultGenerator ();
673+
var info = new AssemblyManifestInfo ();
674+
info.ApplicationProperties = new Dictionary<string, object?> {
675+
["BackupAgent"] = "MyApp.MyBackupAgent",
676+
};
677+
var peers = new List<JavaPeerInfo> {
678+
new JavaPeerInfo {
679+
JavaName = "com/example/app/MyBackupAgent",
680+
CompatJniName = "com/example/app/MyBackupAgent",
681+
ManagedTypeName = "MyApp.MyBackupAgent",
682+
ManagedTypeNamespace = "MyApp",
683+
ManagedTypeShortName = "MyBackupAgent",
684+
AssemblyName = "TestApp",
685+
},
686+
};
687+
688+
var doc = GenerateAndLoad (gen, peers: peers, assemblyInfo: info);
689+
var app = doc.Root?.Element ("application");
690+
Assert.NotNull (app);
691+
Assert.Equal ("com.example.app.MyBackupAgent", (string?)app?.Attribute (AndroidNs + "backupAgent"));
692+
}
693+
694+
[Fact]
695+
public void AssemblyLevel_ApplicationManageSpaceActivity ()
696+
{
697+
var gen = CreateDefaultGenerator ();
698+
var info = new AssemblyManifestInfo ();
699+
info.ApplicationProperties = new Dictionary<string, object?> {
700+
["ManageSpaceActivity"] = "MyApp.ManageActivity",
701+
};
702+
var peers = new List<JavaPeerInfo> {
703+
new JavaPeerInfo {
704+
JavaName = "com/example/app/ManageActivity",
705+
CompatJniName = "com/example/app/ManageActivity",
706+
ManagedTypeName = "MyApp.ManageActivity",
707+
ManagedTypeNamespace = "MyApp",
708+
ManagedTypeShortName = "ManageActivity",
709+
AssemblyName = "TestApp",
710+
},
711+
};
712+
713+
var doc = GenerateAndLoad (gen, peers: peers, assemblyInfo: info);
714+
var app = doc.Root?.Element ("application");
715+
Assert.NotNull (app);
716+
Assert.Equal ("com.example.app.ManageActivity", (string?)app?.Attribute (AndroidNs + "manageSpaceActivity"));
717+
}
619718
}

‎tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/AssemblyAttributeScanningTests.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,21 @@ public void MetaData_ConstructorArgAndNamedArg ()
6666
Assert.NotNull (meta);
6767
Assert.Equal ("test-value", meta.Value);
6868
}
69+
70+
[Fact]
71+
public void UsesPermission_Flags ()
72+
{
73+
var info = ScanAssemblyManifestInfo ();
74+
var perm = info.UsesPermissions.FirstOrDefault (p => p.Name == "android.permission.POST_NOTIFICATIONS");
75+
Assert.NotNull (perm);
76+
Assert.Equal ("neverForLocation", perm.UsesPermissionFlags);
77+
}
78+
79+
[Fact]
80+
public void SupportsGLTexture_ConstructorArg ()
81+
{
82+
var info = ScanAssemblyManifestInfo ();
83+
var gl = info.SupportsGLTextures.FirstOrDefault (g => g.Name == "GL_OES_compressed_ETC1_RGB8_texture");
84+
Assert.NotNull (gl);
85+
}
6986
}

0 commit comments

Comments
 (0)