Skip to content

Commit d64ece1

Browse files
simonrozsivalCopilot
simonrozsival
and
Copilot
committed
[TrimmableTypeMap] Scanner and JCW generator fixes
- CRC64 fix: Use Jones algorithm (Crc64Helper) matching the runtime, not System.IO.Hashing.Crc64 - Inherited override detection: Walk past DoNotGenerateAcw intermediate MCW base types when detecting method overrides - JCW lazy registerNatives: Application/Instrumentation types use deferred __md_registerNatives() helper instead of static initializer - Backtick sanitization: Clean generic arity markers in proxy type names Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d4ebec9 commit d64ece1

File tree

10 files changed

+254
-23
lines changed

10 files changed

+254
-23
lines changed

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

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,15 +122,28 @@ static void WriteClassDeclaration (JavaPeerInfo type, TextWriter writer)
122122

123123
static void WriteStaticInitializer (JavaPeerInfo type, TextWriter writer)
124124
{
125+
string className = JniSignatureHelper.GetJavaSimpleName (type.JavaName);
126+
125127
// Application and Instrumentation types cannot call registerNatives in their
126-
// static initializer — the native library isn't loaded yet at that point.
127-
// Their registerNatives call is emitted in the generated
128-
// ApplicationRegistration.registerApplications() method instead.
128+
// static initializer — the runtime isn't ready yet at that point. Emit a
129+
// lazy one-time helper instead so the first managed callback can register
130+
// the class just before invoking its native method.
129131
if (type.CannotRegisterInStaticConstructor) {
132+
writer.Write ($$"""
133+
private static boolean __md_natives_registered;
134+
private static synchronized void __md_registerNatives ()
135+
{
136+
if (!__md_natives_registered) {
137+
mono.android.Runtime.registerNatives ({{className}}.class);
138+
__md_natives_registered = true;
139+
}
140+
}
141+
142+
143+
""");
130144
return;
131145
}
132146

133-
string className = JniSignatureHelper.GetJavaSimpleName (type.JavaName);
134147
writer.Write ($$"""
135148
static {
136149
mono.android.Runtime.registerNatives ({{className}}.class);
@@ -154,7 +167,17 @@ static void WriteConstructors (JavaPeerInfo type, TextWriter writer)
154167
public {{simpleClassName}} ({{parameters}})
155168
{
156169
super ({{superArgs}});
170+
171+
""");
172+
173+
if (!type.CannotRegisterInStaticConstructor) {
174+
writer.Write ($$"""
157175
if (getClass () == {{simpleClassName}}.class) nctor_{{ctor.ConstructorIndex}} ({{args}});
176+
177+
""");
178+
}
179+
180+
writer.Write ($$"""
158181
}
159182
160183
@@ -197,6 +220,10 @@ static void WriteFields (JavaPeerInfo type, TextWriter writer)
197220

198221
static void WriteMethods (JavaPeerInfo type, TextWriter writer)
199222
{
223+
string registerNativesLine = type.CannotRegisterInStaticConstructor
224+
? "\t\t__md_registerNatives ();\n"
225+
: "";
226+
200227
foreach (var method in type.MarshalMethods) {
201228
if (method.IsConstructor) {
202229
continue;
@@ -222,7 +249,7 @@ static void WriteMethods (JavaPeerInfo type, TextWriter writer)
222249
@Override
223250
public {{javaReturnType}} {{method.JniName}} ({{parameters}}){{throwsClause}}
224251
{
225-
{{returnPrefix}}{{method.NativeCallbackName}} ({{args}});
252+
{{registerNativesLine}} {{returnPrefix}}{{method.NativeCallbackName}} ({{args}});
226253
}
227254
public native {{javaReturnType}} {{method.NativeCallbackName}} ({{parameters}});
228255
@@ -233,7 +260,7 @@ static void WriteMethods (JavaPeerInfo type, TextWriter writer)
233260
234261
{{access}} {{javaReturnType}} {{method.JniName}} ({{parameters}}){{throwsClause}}
235262
{
236-
{{returnPrefix}}{{method.NativeCallbackName}} ({{args}});
263+
{{registerNativesLine}} {{returnPrefix}}{{method.NativeCallbackName}} ({{args}});
237264
}
238265
{{access}} native {{javaReturnType}} {{method.NativeCallbackName}} ({{parameters}});
239266

‎src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/Model/TypeMapAssemblyData.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ sealed record UcoMethodData
182182
/// An [UnmanagedCallersOnly] static wrapper for a constructor callback.
183183
/// Signature must match the full JNI native method signature (jnienv + self + ctor params)
184184
/// so the ABI is correct when JNI dispatches the call.
185-
/// Body: TrimmableNativeRegistration.ActivateInstance(self, typeof(TargetType)).
185+
/// Body: TrimmableTypeMap.ActivateInstance(self, typeof(TargetType)).
186186
/// </summary>
187187
sealed record UcoConstructorData
188188
{

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,9 @@ static JavaPeerProxyData BuildProxyType (JavaPeerInfo peer, HashSet<string> used
173173
{
174174
// Use managed type name for proxy naming to guarantee uniqueness across aliases
175175
// (two types with the same JNI name will have different managed names).
176-
var proxyTypeName = peer.ManagedTypeName.Replace ('.', '_').Replace ('+', '_') + "_Proxy";
176+
// Replace generic arity markers too, because backticks would make the emitted
177+
// proxy type itself look generic even though we don't emit generic parameters.
178+
var proxyTypeName = peer.ManagedTypeName.Replace ('.', '_').Replace ('+', '_').Replace ('`', '_') + "_Proxy";
177179

178180
// Guard against name collisions (e.g., "My.Type" and "My_Type" both map to "My_Type_Proxy")
179181
if (!usedProxyNames.Add (proxyTypeName)) {

‎src/Microsoft.Android.Sdk.TrimmableTypeMap/Microsoft.Android.Sdk.TrimmableTypeMap.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
<TargetFramework>$(TargetFrameworkNETStandard)</TargetFramework>
66
<Nullable>enable</Nullable>
77
<WarningsAsErrors>Nullable</WarningsAsErrors>
8+
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
89
<RootNamespace>Microsoft.Android.Sdk.TrimmableTypeMap</RootNamespace>
910
<SignAssembly>true</SignAssembly>
1011
<AssemblyOriginatorKeyFile>..\..\product.snk</AssemblyOriginatorKeyFile>
@@ -18,6 +19,8 @@
1819

1920
<ItemGroup>
2021
<Compile Include="..\..\src-ThirdParty\System.Runtime.CompilerServices\CompilerFeaturePolyfills.cs" Link="CompilerFeaturePolyfills.cs" />
22+
<Compile Include="..\..\external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\Crc64Helper.cs" Link="Crc64Helper.cs" />
23+
<Compile Include="..\..\external\Java.Interop\src\Java.Interop.Tools.JavaCallableWrappers\Java.Interop.Tools.JavaCallableWrappers\Crc64.Table.cs" Link="Crc64.Table.cs" />
2124
</ItemGroup>
2225

2326
</Project>

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Reflection.Metadata;
77
using System.Reflection.Metadata.Ecma335;
88
using System.Reflection.PortableExecutable;
9+
using Java.Interop.Tools.JavaCallableWrappers;
910

1011
namespace Microsoft.Android.Sdk.TrimmableTypeMap;
1112

@@ -732,7 +733,7 @@ bool TryResolveBaseType (TypeDefinition typeDef, AssemblyIndex index,
732733
(RegisterInfo Info, string DeclaringTypeName, string DeclaringAssemblyName)? FindBaseRegisteredMethodInfo (
733734
TypeDefinition typeDef, AssemblyIndex index, string methodName, MethodDefinition derivedMethod)
734735
{
735-
if (!TryResolveBaseType (typeDef, index, out var baseTypeDef, out var baseHandle, out var baseIndex, out var baseTypeName, out var baseAssemblyName)) {
736+
if (!TryResolveBaseType (typeDef, index, out var baseTypeDef, out _, out var baseIndex, out var baseTypeName, out var baseAssemblyName)) {
736737
return null;
737738
}
738739

@@ -760,10 +761,8 @@ bool TryResolveBaseType (TypeDefinition typeDef, AssemblyIndex index,
760761
}
761762
}
762763

763-
// Recurse up the hierarchy (stop at DoNotGenerateAcw boundary)
764-
if (baseIndex.RegisterInfoByType.TryGetValue (baseHandle, out var baseRegInfo) && baseRegInfo.DoNotGenerateAcw) {
765-
return null;
766-
}
764+
// Keep walking the full base hierarchy so overrides can inherit [Register]
765+
// metadata declared above an intermediate MCW base type.
767766
return FindBaseRegisteredMethodInfo (baseTypeDef, baseIndex, methodName, derivedMethod);
768767
}
769768

@@ -796,7 +795,7 @@ bool TryResolveBaseType (TypeDefinition typeDef, AssemblyIndex index,
796795
MarshalMethodInfo? FindBaseRegisteredProperty (TypeDefinition typeDef, AssemblyIndex index,
797796
string getterName, MethodDefinition derivedGetter)
798797
{
799-
if (!TryResolveBaseType (typeDef, index, out var baseTypeDef, out var baseHandle, out var baseIndex, out var baseTypeName, out var baseAssemblyName)) {
798+
if (!TryResolveBaseType (typeDef, index, out var baseTypeDef, out _, out var baseIndex, out var baseTypeName, out var baseAssemblyName)) {
800799
return null;
801800
}
802801

@@ -835,10 +834,8 @@ bool TryResolveBaseType (TypeDefinition typeDef, AssemblyIndex index,
835834
}
836835
}
837836

838-
// Recurse up (stop at DoNotGenerateAcw boundary)
839-
if (baseIndex.RegisterInfoByType.TryGetValue (baseHandle, out var baseRegInfo) && baseRegInfo.DoNotGenerateAcw) {
840-
return null;
841-
}
837+
// Keep walking the full base hierarchy so property overrides can inherit
838+
// [Register] metadata declared above an intermediate MCW base type.
842839
return FindBaseRegisteredProperty (baseTypeDef, baseIndex, getterName, derivedGetter);
843840
}
844841

@@ -1477,8 +1474,11 @@ static string GetCrc64PackageName (string ns, string assemblyName)
14771474
return ns.ToLowerInvariant ().Replace ('.', '/');
14781475
}
14791476

1477+
// Keep this in sync with JavaNativeTypeManager.ToJniName(Type)/(TypeDefinition).
1478+
// The trimmable build path must emit the exact same CRC64 package names that the
1479+
// runtime later computes for FindClass(Type) and peer activation.
14801480
var data = System.Text.Encoding.UTF8.GetBytes ($"{ns}:{assemblyName}");
1481-
var hash = System.IO.Hashing.Crc64.Hash (data);
1481+
var hash = Crc64Helper.Compute (data);
14821482
return $"crc64{BitConverter.ToString (hash).Replace ("-", "").ToLowerInvariant ()}";
14831483
}
14841484

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

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,20 @@ public void Generate_AcwType_HasRegisterNativesStaticBlock ()
138138
public void Generate_ApplicationType_SkipsRegisterNatives ()
139139
{
140140
var java = GenerateFixture ("my/app/MyApplication");
141-
Assert.DoesNotContain ("registerNatives", java);
142141
Assert.DoesNotContain ("static {", java);
142+
Assert.DoesNotContain ("if (getClass () == MyApplication.class) nctor_0 ();", java);
143+
AssertContainsLine ("private static synchronized void __md_registerNatives ()\n", java);
144+
AssertContainsLine ("mono.android.Runtime.registerNatives (MyApplication.class);\n", java);
143145
}
144146

145147
[Fact]
146148
public void Generate_InstrumentationType_SkipsRegisterNatives ()
147149
{
148150
var java = GenerateFixture ("my/app/MyInstrumentation");
149-
Assert.DoesNotContain ("registerNatives", java);
150151
Assert.DoesNotContain ("static {", java);
152+
Assert.DoesNotContain ("if (getClass () == MyInstrumentation.class) nctor_0 ();", java);
153+
AssertContainsLine ("private static synchronized void __md_registerNatives ()\n", java);
154+
AssertContainsLine ("mono.android.Runtime.registerNatives (MyInstrumentation.class);\n", java);
151155
}
152156

153157
}
@@ -257,6 +261,61 @@ public void Generate_MarshalMethod_HasOverrideAndNativeDeclaration ()
257261
AssertContainsLine ("public native void n_OnCreate_Landroid_os_Bundle_ (android.os.Bundle p0);\n", java);
258262
}
259263

264+
[Fact]
265+
public void Generate_OverrideAcrossIntermediateMcwBase_HasMethodStub ()
266+
{
267+
var java = GenerateFixture ("my/app/SelectableList");
268+
AssertContainsLine ("@Override\n", java);
269+
AssertContainsLine ("public void setSelection (int p0)\n", java);
270+
AssertContainsLine ("n_SetSelection_I (p0);\n", java);
271+
AssertContainsLine ("public native void n_SetSelection_I (int p0);\n", java);
272+
}
273+
274+
[Fact]
275+
public void Generate_OverrideAcrossGenericIntermediateMcwBase_HasMethodStub ()
276+
{
277+
var java = GenerateFixture ("my/app/GenericSelectableList");
278+
AssertContainsLine ("@Override\n", java);
279+
AssertContainsLine ("public void setSelection (int p0)\n", java);
280+
AssertContainsLine ("n_SetSelection_I (p0);\n", java);
281+
AssertContainsLine ("public native void n_SetSelection_I (int p0);\n", java);
282+
}
283+
284+
[Fact]
285+
public void Generate_DeferredRegistrationType_LazilyRegistersBeforeNativeCallback ()
286+
{
287+
var type = new JavaPeerInfo {
288+
JavaName = "my/app/DeferredInstrumentation",
289+
CompatJniName = "my/app/DeferredInstrumentation",
290+
ManagedTypeName = "MyApp.DeferredInstrumentation",
291+
ManagedTypeNamespace = "MyApp",
292+
ManagedTypeShortName = "DeferredInstrumentation",
293+
AssemblyName = "App",
294+
BaseJavaName = "android/app/Instrumentation",
295+
CannotRegisterInStaticConstructor = true,
296+
MarshalMethods = new List<MarshalMethodInfo> {
297+
new () {
298+
JniName = "onCreate",
299+
JniSignature = "(Landroid/os/Bundle;)V",
300+
ManagedMethodName = "OnCreate",
301+
NativeCallbackName = "n_OnCreate_Landroid_os_Bundle_",
302+
Connector = "GetOnCreate_Landroid_os_Bundle_Handler",
303+
},
304+
new () {
305+
JniName = "onStart",
306+
JniSignature = "()V",
307+
ManagedMethodName = "OnStart",
308+
NativeCallbackName = "n_OnStart",
309+
Connector = "GetOnStartHandler",
310+
},
311+
},
312+
};
313+
314+
var java = GenerateToString (type);
315+
AssertContainsLine ("__md_registerNatives ();\n\t\tn_OnCreate_Landroid_os_Bundle_ (p0);\n", java);
316+
AssertContainsLine ("__md_registerNatives ();\n\t\tn_OnStart ();\n", java);
317+
}
318+
260319
}
261320

262321
public class NestedType

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,13 @@ public void Generate_GenericType_ThrowsNotSupportedException ()
199199
using var pe = new PEReader (stream);
200200
var reader = pe.GetMetadataReader ();
201201
var typeNames = GetTypeRefNames (reader);
202+
var generatedTypeNames = reader.TypeDefinitions
203+
.Select (h => reader.GetTypeDefinition (h))
204+
.Select (t => reader.GetString (t.Name))
205+
.ToList ();
202206
Assert.Contains ("NotSupportedException", typeNames);
207+
Assert.Contains ("MyApp_Generic_GenericHolder_1_Proxy", generatedTypeNames);
208+
Assert.DoesNotContain (generatedTypeNames, name => name.Contains ('`'));
203209
}
204210

205211
[Fact]

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
using System.Collections.Generic;
33
using System.IO;
44
using System.Linq;
5+
using System.Reflection;
6+
using System.Text;
7+
using Java.Interop.Tools.JavaCallableWrappers;
58
using Xunit;
69

710
namespace Microsoft.Android.Sdk.TrimmableTypeMap.Tests;
@@ -87,4 +90,28 @@ public void Scan_RegisterAttribute_DotFormat_NormalizedToSlashes ()
8790
Assert.False (peer.DoNotGenerateAcw);
8891
Assert.True (peer.IsUnconditional, "Should be unconditional due to [Activity]");
8992
}
93+
94+
[Theory]
95+
[InlineData ("MyApp.PlainActivitySubclass")]
96+
[InlineData ("MyApp.UnregisteredClickListener")]
97+
[InlineData ("MyApp.UnregisteredExporter")]
98+
public void Scan_UnregisteredType_MatchesJavaNativeTypeManager (string managedName)
99+
{
100+
var testAssemblyDir = Path.GetDirectoryName (typeof (FixtureTestBase).Assembly.Location)
101+
?? throw new InvalidOperationException ("Cannot determine test assembly directory");
102+
var fixtureAssemblyPath = Path.Combine (testAssemblyDir, "TestFixtures.dll");
103+
var fixtureAssembly = Assembly.LoadFrom (fixtureAssemblyPath);
104+
var fixtureType = fixtureAssembly.GetType (managedName);
105+
if (fixtureType is null) {
106+
throw new InvalidOperationException ($"Could not load fixture type '{managedName}' from '{fixtureAssemblyPath}'.");
107+
}
108+
109+
var assemblyName = fixtureType.Assembly.GetName ().Name
110+
?? throw new InvalidOperationException ($"Could not determine assembly name for '{managedName}'.");
111+
var data = Encoding.UTF8.GetBytes ($"{fixtureType.Namespace}:{assemblyName}");
112+
var hash = Crc64Helper.Compute (data);
113+
var expectedJavaName = $"crc64{BitConverter.ToString (hash).Replace ("-", "").ToLowerInvariant ()}/{fixtureType.Name}";
114+
115+
Assert.Equal (expectedJavaName, FindFixtureByManagedName (managedName).JavaName);
116+
}
90117
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,24 @@ public void MultipleOverloads_PicksCorrectOne ()
102102
Assert.DoesNotContain (nonCtorMethods, m => m.JniName == "process" && m.JniSignature == "()V");
103103
}
104104

105+
[Fact]
106+
public void OverrideAcrossIntermediateMcwBase_Detected ()
107+
{
108+
var peer = FindFixtureByJavaName ("my/app/SelectableList");
109+
var setSelection = Assert.Single (peer.MarshalMethods, m => m.JniName == "setSelection");
110+
Assert.Equal ("(I)V", setSelection.JniSignature);
111+
Assert.Equal ("GetSetSelection_IHandler", setSelection.Connector);
112+
}
113+
114+
[Fact]
115+
public void OverrideAcrossGenericIntermediateMcwBase_Detected ()
116+
{
117+
var peer = FindFixtureByJavaName ("my/app/GenericSelectableList");
118+
var setSelection = Assert.Single (peer.MarshalMethods, m => m.JniName == "setSelection");
119+
Assert.Equal ("(I)V", setSelection.JniSignature);
120+
Assert.Equal ("GetSetSelection_IHandler", setSelection.Connector);
121+
}
122+
105123
[Fact]
106124
public void EmptyConnector_OverrideStillDetected ()
107125
{

0 commit comments

Comments
 (0)