JCW Implementor types "pollute" original package

[jonpryor]

Consider a xamarin/xamarin-android build, after which the following file may exist:

src/Mono.Android/obj/Debug/net8.0/android-34/jcw/src/android/hardware/Camera_IOnZoomChangeListenerImplementor.java

This comes from the C# binding:

namespace Android.Hardware {
	public partial class Camera : Java.Lang.Object {
		// Note: no [Register]
		internal sealed partial class IOnZoomChangeListenerImplementor : global::Java.Lang.Object, IOnZoomChangeListener {
		}
	}
}

which results in the Java Callable Wrapper:

package android.hardware;

public class Camera_IOnZoomChangeListenerImplementor
  extends java.lang.Object
  implements
    mono.android.IGCUserPeer,
    android.hardware.Camera.OnZoomChangeListener
{
    // …
}

…and therein lies the issue: why is Camera.IOnZoomChangeListenerImplementor resulting in the Java Callable Wrapper of android.hardware.Camera_IOnZoomChangeListenerImplementor, and not crc64….Camera_IOnZoomChangeListenerImplementor? Why are we "polluting" the original package name?

[jpobst]

Note that this seems to be a side effect of:
#1105

These implementor classes moved from (ex): mono.android.view to android.view.

We are reverting this in:
dotnet/android#8337

It may be desirable to hide them even further into something like crc64..., but if this path is pursued we definitely need to keep the needed Proguard changes in mind as well.

[jonpryor]

@jpobst wrote:

we definitely need to keep the needed Proguard changes in mind as well.

Related: we currently have code to create a ProGuard config file based on TypeDefinitions: https://github.com/xamarin/xamarin-android/blob/41aaf3873f702d260eb20b354ffdbb8884c51984/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/GenerateProguardConfiguration.cs#L74-L75

However, the above code doesn't appear to be invoked in dotnet/android#8337. If it were invoked, then we'd have a -keep class android.view.View_IOnClickListenerImplementor running around, thus "solving" dotnet/android#8337.

For .NET 8, we'll partially revert #1105 so that [Register] is once again emitted, which will "fix" dotnet/android#8337, because the [Register] values will allow this line from proguard_xamarin.cfg to apply, ensuring that ProGuard preserves View_IOnClickListenerImplementor:

https://github.com/xamarin/xamarin-android/blob/main/src/Xamarin.Android.Build.Tasks/Resources/proguard_xamarin.cfg#L10

-keep class mono.android.** { *; <init>(...); }

For .NET 9, we should re-introduce #1105 and address #1132 (this issue) and ensure that xamarin-android emits a ProGuard file which preserves e.g. crc64….View_IOnClickListenerImplementor.

[jonpryor]

Now, why should we care about [Register] ? Mostly because of https://github.com/xamarin/monodroid/commit/eb04c91c:

[MSBuild] Use an MD5SUM for package name, not namespace name.

Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=15205#c1
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=16805#c0
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=16826
Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=21147

The problem is that when generating Android Callable Wrappers the
namespace name was assumed to be unique. (I mean, who would share
source code between assemblies, source code that subclasses
Java.Lang.Object?! Azure Mobile Services, that's who!)

For example:

// Lib1.dll
namespace Utilities {
	class Holder<T> : Java.Lang.Object {
		public T Value { get; set; }
	}
}
// Lib2.dll
namespace Utilities {
	class Holder<T> : Java.Lang.Object {
		public T Value { get; set; }
	}
}

In the above example, previous releases would lowercase the namespace
name to create the package name, while the class name is unchanged.
The result is that both types have the same ACW name:
utilities.Holder. This in turn causes a build-time crash, due to the
duplicate names, e.g.

error : Duplicate managed type found! Mappings between managed types and Java types must be unique.
First Type: 'Android.Support.V4.App.FragmentManager/IOnBackStackChangedListenerImplementor, Xamarin.Android.Support.v4-r18, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null';
Second Type: 'Android.Support.V4.App.FragmentManager/IOnBackStackChangedListenerImplementor, Mono.Android.Support.v4, Version=0.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065'

To remove this problem, we instead use a package name which is the
"md5" + MD5SUM("@namespace@, @AssemblyQualifiedName@"). This ensures
that different assemblies get unique package names, even if their
namespace is the same. (The "md5" prefix is to ensure that the first
character is not a digit, and is thus a valid Java package identifier.)

Rephrased: "infrastructure" types such as IOnClickListenerImplementor "shouldn't be allowed to collide" with identically named types from other assemblies, unless there's a good reason to prevent that. Emitting [Register] on such types can introduce Java-side collisions for otherwise "valid" constructs, and thus @jonpryor would prefer that [Register] not be emitted unless otherwise required (e.g. bindings, or "user-friendly" Java-side activity names for adb shell am start …).

[jonpryor]

@jpobst also determined why JCW's were "polluting" the original package:

https://github.com/xamarin/java.interop/blob/8e63cc8da1882cadc7db329bfa6cafb6f914769f/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs#L200-L209

If the type does not have [Register] attribute, and the type is within Mono.Android.dll, we lowercase the namespace name to determine the package name for the Java Callable Wrapper.

Which explains why the Java Callable Wrapper for Android.Hardware.Camera.IOnZoomChangeListenerImplementor is android.hardware.Camera_IOnZoomChangeListenerImplementor.

(Though that doesn't quite explain ed63d89 and android.view.View_IOnClickListenerImplementor; shouldn't that be android.views.View_IOnClickListenerImplementor, as the namespace name is Android.Views?)

(See also: https://github.com/xamarin/monodroid/commit/bb245a48c8d8f6578d1aae8ecd6cfa64b29105b4 )