Fix legacy generic dictionary conversion

[simonrozsival]

Summary

Experimental coverage of all 400 key/value combinations across primitive, nullable primitive, string, and Java peer types showed that JavaConvert.FromJniHandle(Type) produces the exact requested JavaDictionary<TKey,TValue>, while reflection-backed value managers reused a cached non-generic JavaDictionary and GetValue<T> then failed with InvalidCastException.

This change makes both legacy reflection-backed Android value managers honor explicit closed IDictionary<TKey,TValue> and JavaDictionary<TKey,TValue> targets by routing them through the existing JavaConvert generic collection factory. It also teaches the dynamic factory path to accept an explicit JavaDictionary<TKey,TValue> target.

Compatibility

• Java-to-managed default type inference is unchanged because only explicit closed generic dictionary targets take the new path.
• Non-generic IDictionary conversion remains unchanged.
• JavaConvert returns an already cached peer when it is assignable to the requested type, preserving peer identity.
• The implementation uses the existing reflection-backed generic collection factory and introduces no new arbitrary trimming roots.

Tests

Added representative device coverage for:

• primitive/string IDictionary<int, string>
• nullable primitive JavaDictionary<int?, double?>
• string/Java peer IDictionary<string, MyIntent>
• Java peer/nullable primitive JavaDictionary<MyIntent, long?>
• non-generic dictionary behavior
• assignable cached generic peer identity

Validation:

• make all CONFIGURATION=Debug
• dotnet build src/Mono.Android/Mono.Android.csproj -c Debug --no-restore
• dotnet test external/Java.Interop/tests/Java.Interop-Tests/Java.Interop-Tests.csproj -c Debug (670 passed, 6 skipped)
• on-device Mono.Android.NET-Tests (918 passed, 55 skipped)

The 400-case result is the experimental motivating matrix; the committed regression suite intentionally uses representative shapes rather than duplicating all 400 combinations.

[simonrozsival]

@copilot resolve the merge conflicts in this pull request

[Copilot]

@copilot resolve the merge conflicts in this pull request

Resolved in 8242416.

[simonrozsival]

@copilot resolve the merge conflicts in this pull request

[Copilot]

@copilot resolve the merge conflicts in this pull request

Resolved in b695434.

[Copilot]

Pull request overview

Fixes legacy reflection-backed JniValueManager dictionary conversions so that explicit closed generic dictionary targets (IDictionary<TKey,TValue> and JavaDictionary<TKey,TValue>) are routed through JavaConvert’s generic-collection conversion, avoiding cached non-generic JavaDictionary reuse that can lead to InvalidCastException.

Changes:

• Route explicit generic dictionary targets through JavaConvert.FromObjectReference(...) in both legacy reflection-backed value managers (AndroidValueManager and JavaMarshalValueManager).
• Extend the dynamic generic-collection factory path to accept explicit JavaDictionary<,> targets (in addition to IDictionary<,>).
• Add device regression tests covering representative generic/non-generic dictionary conversion shapes and cached-peer identity behavior.

Show a summary per file

File	Description
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs	Adds regression coverage for generic dictionary conversions and cached-peer identity behavior.
src/Mono.Android/Microsoft.Android.Runtime/JavaMarshalValueManager.cs	Ensures reflection-backed CoreCLR value manager routes explicit generic dictionary targets through JavaConvert.
src/Mono.Android/Java.Interop/JavaConvert.cs	Expands generic-collection factory support to include JavaDictionary<,> targets and adds a helper to detect explicit generic dictionary types.
src/Mono.Android/Android.Runtime/AndroidRuntime.cs	Ensures legacy reflection-backed Android value manager routes explicit generic dictionary targets through JavaConvert.

Review details

Suppressed comments (1)

tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs:163

• 💡 Here new JniObjectReference (source.Handle) uses the default JniObjectReferenceType.Invalid. Using var reference = source.PeerReference; makes the reference type accurate (global) and avoids subtle ownership issues if this test ever switches to a disposing/transfer option.

 using (var source = new JavaDictionary ()) {
source.Add (key, value);
var reference = new JniObjectReference (source.Handle);
var actual = JniEnvironment.Runtime.ValueManager.GetValue (
ref reference, JniObjectReferenceOptions.Copy, targetType);

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Lite

[simonrozsival]

Closing in favor of #12677.

While reviving this PR it became clear the scope here is narrower than the actual bug. ReflectionJniValueManager.GetValueMarshalerCore only special-cases IList<> via GetListType, so ICollection<T> falls through to ProxyValueMarshaler for exactly the same reason IDictionary<K,V> does — and ISet<T>/JavaSet<T> is missing from JavaConvert and SafeJavaCollectionFactory entirely. Fixing only dictionaries would leave the sibling cases behind under a predicate named for just one of them.

The GetValueCore overrides here also sit at the wrong layer: they run ahead of the base class's EnsureNotDisposed(), reference.IsValid, PeekValue, and targetType/T compatibility checks, and the same block is duplicated in AndroidValueManager and JavaMarshalValueManager. The pattern from #12114 (a protected virtual hook in ReflectionJniValueManager, as done for CreateNonArrayListValue) is a better fit and would cover both managers at once.

Worth recording for whoever picks up #12677: this was never a regression. The reflection path has always had this hole on both MonoVM and CoreCLR; the trimmable typemap manager avoids it only because it routes everything through JavaConvert.FromObjectReference, and its behavior is the reference for what the reflection path should do.

The dictionary fix and the on-device tests on this branch are still good starting material — the branch is kept for reuse.