[Mono.Android] Enumify API-36.1, API-36.1 is "Stable" (#10515)

Context: http://github.com/jpobst/BindingStudio
Context: #10005

Use jpobst/BindingStudio to begin enumifying API-36.1.

Note: current dotnet/java-interop emits an "extra" `,` on `map.csv`
output, which would make for a "noisy" diff (every line changed!).
The diff size is reduced by removing trailing commas:

	sed 's/,$//' < src/Mono.Android/new-map.csv > src/Mono.Android/map.csv

This keeps the diff to a reasonable size.

TODO: update `map.csv` to current BindingStudio output after this
is merged.

Update `build-tools/manifest-attribute-codegen` so that:

 1. It can be executed from topdir.

 2. Warning message are actually emitted when using `dotnet build`.
    Previously, only the fact that the executed command failed would
    be printed, but none of the tool output, which was needed!

~~ Stable API-36.1 ~~

Normally when we declare a new API level as "stable", it is the
*only* API that is built.

For API-36.1, this is not the case: we want to continue building
API-36, *and also* build API-36.1, *and also* using the the API-36.1
build artifacts doesn't require `$(EnablePreviewFeatures)`=true.

Mirror #10005:

  * build both API levels by "repurposing" the
    `$(AndroidLatestUnstable*)` MSBuild properties as an
    "alternate set" of stable API levels.

  * What "really" indicates a `Mono.Android.dll` is unstable is the
    presence of `ANDROID_UNSTABLE` when building it.
    This is conditional on `$(IsUnstableVersion)`.

    Update `$(IsUnstableVersion)` so that it uses the
    [MSBuild version-comparison functions][0] to compare
    `$(AndroidApiLevel)` against `$(AndroidLatestStableApiLevel)`
    (previous behavior) *and also* against
    `$(AndroidLatestStableApiLevel2)` (new property).

    This allows API-36.1 to be built via the `*Unstable*` properties
    while not defining `ANDROID_UNSTABLE.`

~~ Fix assertion failure in <CheckApiCompatibility/> ~~

When attempting to declare API-36.1 as the latest stable API,
my local (Debug) build promptly started *crashing*.

Which was rather unexpected.

The most useful lesson in this context: *do not* call
`Debug.Assert()` or throw exceptions from code which is called from
the Thread Pool.  `Debug.Assert()` -- in Debug config builds -- will
immediately exit the process, which in turn means that MSBuild `binlog`
output is *incomplete*, as MSBuild never gets a chance to flush
everything (courtesy of the process aborting!).

Throwing an exception is very similar: unhandled exceptions on the
thread pool cause the process to abort.  MSBuild never has a chance
to write complete binlog data.

Plumb logging through to `CodeGenDiff` so that the assertion failure
can instead become an "Internal Error" message.  This fixes the crash.

Then there's the matter of the former assertion failure now error
message: why was `currentObject.InternalCounter` negative?

`currentObject.InternalCounter` was negative because of this line
(newlines added for readability, but it's all one line):

	public static void AddEventHandler<TInterface, TImplementor>(
	  [System.Runtime.CompilerServices.NullableAttribute((byte)2)] ref System.WeakReference? implementor,
	  System.Func<TImplementor> creator,
	  System.Action<TInterface> setListener,
	  System.Action<TImplementor> add)
	where TInterface : class
	where TImplementor : Java.Lang.Object, TInterface
	{
	}

The `where TInterface : class ` was matching this check:

        content.IndexOf (" class ", StringComparison.Ordinal)

and thus the method was being interpreted as the introduction of
a new type declaration instead of something to add to the existing
`currentObject` scope.  This meant that the new
`currentObject.InternalCounter` was 0, then we hit `}` to decrement
`.InternalCounter`, resulting in a negative value.

Fix this by checking for ` partial class `, as was done previously
with ` partial struct `, as `Microsoft.DotNet.GenAPI.dll` always
emits the `partial` modifier on everything except `enum`s.
This fixes the now internal error message former assertion.

[0]: https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions?iew=vs-2022#msbuild-version-comparison-functions

Author: jonpryor

Configuration.props

@@ -28,9 +28,10 @@
     <AndroidLatestStableApiLevel Condition="'$(AndroidLatestStableApiLevel)' == ''">36</AndroidLatestStableApiLevel>
     <AndroidLatestStablePlatformId Condition="'$(AndroidLatestStablePlatformId)' == ''">$(AndroidLatestStableApiLevel)</AndroidLatestStablePlatformId>
     <AndroidLatestStableFrameworkVersion Condition="'$(AndroidLatestStableFrameworkVersion)'==''">v16.0</AndroidLatestStableFrameworkVersion>
+    <AndroidLatestStableApiLevel2 Condition="'$(AndroidLatestStableApiLevel2)' == ''">36.1</AndroidLatestStableApiLevel2>
     <!-- *Latest* *unstable* API level binding that we support; this can be the same as *stable* -->
     <AndroidLatestUnstableApiLevel Condition="'$(AndroidLatestUnstableApiLevel)' == ''">36.1</AndroidLatestUnstableApiLevel>
-    <AndroidLatestUnstablePlatformId Condition="'$(AndroidLatestUnstablePlatformId)' == ''">36.1</AndroidLatestUnstablePlatformId>
+    <AndroidLatestUnstablePlatformId Condition="'$(AndroidLatestUnstablePlatformId)' == ''">$(AndroidLatestUnstableApiLevel)</AndroidLatestUnstablePlatformId>
     <AndroidLatestUnstableFrameworkVersion Condition="'$(AndroidLatestUnstableFrameworkVersion)'==''">v16.1</AndroidLatestUnstableFrameworkVersion>
     <!-- The default API level used for $(TargetPlatformVersion) -->
     <AndroidDefaultTargetDotnetApiLevel Condition=" '$(AndroidDefaultTargetDotnetApiLevel)' == '' ">$(AndroidLatestStableApiLevel)</AndroidDefaultTargetDotnetApiLevel>

Documentation/workflow/HowToAddNewApiLevel.md

@@ -6,6 +6,8 @@ The first unstable preview generally ships in late February or early March.  At
 stage for the APIs, we simply add literal bindings for them.  We do not spend resources on
 the more manual parts like enumification that will likely change as the APIs mature.
 
+<a name="repository-xml"></a>
+
 ### Review `repository2-3.xml`
 
 <https://dl.google.com/android/repository/repository2-3.xml> is an XML description of the Android SDK,
@@ -256,6 +258,104 @@ It's a Winforms app, so it only runs on Windows.  It's ugly as sin, and has very
 it prompts you with the exact decisions you need to make, and handles as much dirty work as possible,
 allowing enumification to be done in a few days.
 
+### Source of Inspiration for enum grouping
+
+*If the Android sources package has been published* -- which is not always a given, and thus why
+historically this could not be relied upon for automated enumification -- then the Android Source
+package can be used as a "source of inspiration".  The Android Source package is listed in the
+<a href="#repository-xml">repository XML</a> in a "source" remote package:
+
+```xml
+<sdk:sdk-repository …>
+  <remotePackage path="sources;android-36.1">
+    <type-details xsi:type="sdk:sourceDetailsType">
+      <api-level>36.1</api-level>
+      <extension-level>20</extension-level>
+      <base-extension>true</base-extension>
+    </type-details>
+    <revision>
+      <major>1</major>
+    </revision>
+    <display-name>Sources for Android 36.1</display-name>
+    <uses-license ref="android-sdk-license"/>
+    <channelRef ref="channel-0"/>
+    <archives>
+      <archive>
+        <complete>
+          <size>51810808</size>
+          <checksum type="sha1">54cea0371ec284404e06051cd389646d34470da4</checksum>
+          <url>source-36.1_r01.zip</url>
+        </complete>
+      </archive>
+    </archives>
+  </remotePackage>
+</sdk:sdk-repository>
+```
+
+As with other Repository packages, append the `//archive/complete/url` value to
+`https://dl.google.com/android/repository/`, creating e.g.
+<https://dl.google.com/android/repository/source-36.1_r01.zip>.  The contents of
+this archive is *Java source code* for the public Android API.
+
+Within the Java source code is usage of *source-only annotations* which Java IDEs
+can use to associate Java constants with method parameters and return types.
+For example, from `src/android/view/View.java`:
+
+```java
+public /* partial */ class View {
+
+    @FlaggedApi(FLAG_REQUEST_RECTANGLE_WITH_SOURCE)
+    public boolean requestRectangleOnScreen(@NonNull Rect rectangle, boolean immediate,
+            @RectangleOnScreenRequestSource int source) {
+        …
+    }
+
+    /**
+     * @hide
+     */
+    @IntDef(prefix = { "RECTANGLE_ON_SCREEN_REQUEST_SOURCE_" }, value = {
+            RECTANGLE_ON_SCREEN_REQUEST_SOURCE_UNDEFINED,
+            RECTANGLE_ON_SCREEN_REQUEST_SOURCE_SCROLL_ONLY,
+            RECTANGLE_ON_SCREEN_REQUEST_SOURCE_TEXT_CURSOR,
+            RECTANGLE_ON_SCREEN_REQUEST_SOURCE_INPUT_FOCUS,
+    })
+    @Retention(RetentionPolicy.SOURCE)
+    public @interface RectangleOnScreenRequestSource {}
+
+    @FlaggedApi(FLAG_REQUEST_RECTANGLE_WITH_SOURCE)
+    public static final int RECTANGLE_ON_SCREEN_REQUEST_SOURCE_UNDEFINED = 0x00000000;
+
+    @FlaggedApi(FLAG_REQUEST_RECTANGLE_WITH_SOURCE)
+    public static final int RECTANGLE_ON_SCREEN_REQUEST_SOURCE_SCROLL_ONLY = 0x00000001;
+
+    @FlaggedApi(FLAG_REQUEST_RECTANGLE_WITH_SOURCE)
+    public static final int RECTANGLE_ON_SCREEN_REQUEST_SOURCE_TEXT_CURSOR = 0x00000002;
+
+    @FlaggedApi(FLAG_REQUEST_RECTANGLE_WITH_SOURCE)
+    public static final int RECTANGLE_ON_SCREEN_REQUEST_SOURCE_INPUT_FOCUS = 0x00000003;
+}
+```
+
+The `public @interface RectangleOnScreenRequestSource` introduces a *Java annotation*.
+The `@RectangleOnScreenRequestSource` annotation itself has
+[`@Retention(RetentionPolicy.SOURCE)`](https://developer.android.com/reference/kotlin/java/lang/annotation/Retention.html),
+which indicates that the annotation will only be present in *source code*, not `.class` files.
+The `@RectangleOnScreenRequestSource` annotation also has an
+[`@IntDef` annotation](https://developer.android.com/reference/androidx/annotation/IntDef), which is
+used to specify which constants are grouped together.
+
+  * `IntDef.prefix` is an optional string listing a common prefix for all the grouped constants.
+  * `IntDef.flag` is an optional boolean value specifying whether the grouped constants
+    are usable in a bitwise context, akin to `[System.Flags]`.
+  * `IntDef.value` is an array of the grouped constants.
+
+The `@RectangleOnScreenRequestSource` annotation can then be used on Java method parameters
+and return types to indicate which parameters and return types should be enumified.
+
+Something to consider for the future: *if* Google reliably provides source packages,
+these annotations could be used to auto-generate enumified APIs.  For now, this information
+can be used as a source of inspiration for enum names and what should be in the enums.
+
 ### Extract constants from API
 
 Using BindingStudio:
@@ -327,12 +427,15 @@ The left tree view can be updated by saving and reopening the `map.csv` file.
 
 Using BindingStudio:
 
-- Update the file paths in `MainForm.FindAPILevelMethodsToolStripMenuItem_Click`
+- Update the file paths in `MainForm.FindAPILevelMethodsToolStripMenuItem_Click`.
+  In particular, `api` is the path to the `api-*.xml` file to parse, and
+  `csv` is the name of a CSV file containing the contents described below.
 - Run BindingStudio and choose `Tools` -> `Find API Level Methods`
 
-This will create a file of every method in the new API level that takes an `int` as a parameter
-or returns an `int` as a return value.  Each method will be marked with a `?` in the file
-to indicate a decision needs to be made to ignore it or map it to an enum.
+The `csv` variable within `MainForm.FindAPILevelMethodsToolStripMenuItem_Click()` is the name
+of the created file which contains every method in the new API level that takes an `int` as a
+parameter or returns an `int` as a return value.  Each method will be marked with a `?` in the
+file to indicate a decision needs to be made to ignore it or map it to an enum.
 
 Example:
 ```
@@ -347,7 +450,7 @@ Using BindingStudio:
 - Choose `File` -> `Open Constant Map`
   - Choose existing `map.csv`: `xamarin-android/src/Mono.Android/map.csv`
 - Choose `File` -> `Open Method Map`
-  - Choose the new `.csv` created in the previous step
+  - Choose the new `.csv` created in the previous step.
 
 The left tree will populate with every method that possibly should be enumified and
 needs a decision to be made.  Clicking a method shows the Android documentation for
@@ -358,45 +461,87 @@ Note a method may show up multiple times, once for each parameter or return type
 
 There are 3 possible options for a method parameter/return type:
 
-1) Unknown
+ 1. Unknown
 
-You don't how to handle this method currently, so leaving it in the initial state
-of "Unknown" will leave it alone until a decision can be made.
+    You don't how to handle this method currently, so leaving it in the initial
+    state of "Unknown" will leave it alone until a decision can be made.
 
-2) Ignore
+ 2. Ignore
 
-The method parameter/return type should remain an `int` and not be converted to an enum.
+    The method parameter/return type should remain an `int` and not be converted to an enum.
 
-Ex: 
-```
-int Add (int value1, int value2) { ... }
-```
+    For example:
+
+    ```java
+    int Add (int value1, int value2) { ... }
+    ```
+
+    Click the **Ignore** radio button and then the **Save** button.
+
+ 3. Enumify
 
-Click the "Ignore" radio button and then the "Save" button.
+    The method parameter/return type should be changed to an enum.
+
+    For example:
+
+    ```java
+    void AudioAttributesBuilder.SetSpatializationBehavior (int sb) {… }
+    ```
+
+    - Choose the **Enumify** radio option
+    - Use the DropDown in the bottom-left to select the enum to use
+      - When selected, the members of that enum will be shown in the box below the enum
+    - Alternatively, search for a enum by enum member name using the Search box in the right
+      - If desired enum is found, clicking it will populate dropdown
+    - Click **Save**
+
+Use `File` -> `Save` to save your work often!
 
-3) Enumify
+### Turning `int` into `Color`
 
-The method parameter/return type should be changed to an enum.
+As part of the above **Mapping methods** process, some methods may appear which should
+be an `Android.Graphics.Color`, e.g. methods named `getTextColor()`.
 
-Ex:
+[`src/Mono.Android/metadata`](../../src/Mono.Android/metadata) is used to transform `int`
+parameter and return types into `Android.Graphics.Color`.  This is a manual process.
+
+To map the return type:
+
+```xml
+<attr
+    api-since="36.1"
+    path="/api/package[@name='…package name…']/class[@name='…class name…]/method[@name='…method name…']"
+    name="return"
+>Android.Graphics.Color</attr>
 ```
-void AudioAttributesBuilder.SetSpatializationBehavior (int sb) { ... }
+
+Specifically:
+
+  * `//attr/@path` is the XPath expression to the method name to update
+  * `//attr/@name` the XML attribute to update.  For return types, this is `return`.
+  * The value of the `<attr/>` is `Android.Graphics.Color`.
+
+To map parameter types:
+
+```xml
+<attr
+    api-since="36.1"
+    path="/api/package[@name='…package name…']/class[@name='…class name…]/method[@name='…method name…']/parameter[@type='int']"
+  name="type"
+>Android.Graphics.Color</attr>
 ```
 
-- Choose the "Enumify" radio option
-- Use the DropDown in the middle to select the enum to use
-  - When selected, the members of that enum will be shown in the box below the enum
-- Alternatively, search for a enum by enum member name using the Search box in the right
-  - If desired enum is found, clicking it will populate dropdown
-- Click "Save"
+  * `//attr/@path` is the XPath expression to the parameter to update
+  * `//attr/@name` the XML attribute to update.  For return types, this is `type`.
+  * The value of the `<attr/>` is `Android.Graphics.Color`.
 
-Use `File` -> `Save` to save your work often!
 
 ### Finishing the method map
 
 The official `methodmap.csv` uses a slightly different format than the one used for enumification.
 
 Using BindingStudio:
+
 - Ensure the "new api level method map" CSV file is loaded.
 - Choose `Tools` -> `Export Final Method Map`
 - Choose a temporary file name
@@ -405,26 +550,41 @@ Using BindingStudio:
 
 Congrats! Enumification is complete!
 
----- Somewhat outdated docs below, update when we do this year's stabilization ----
+But wait, there's more!
+
+### New `AndroidManifest.xml` elements and attributes
 
-6) new AndroidManifest.xml elements and attributes
+`build-tools/manifest-attribute-codegen/manifest-attribute-codegen.cs` can be
+compiled to a tool that collects all Manifest elements and attributes with the
+API level since when each of them became available.  New members are supposed
+to be added to the existing `(FooBar)Attribute.cs` and
+`(FooBar)Attribute.Partial.cs` in `src/Mono.Android` and
+`src/Xamarin.Android.Build.Tasks`, respectively.
 
-`build-tools/manifest-attribute-codegen/manifest-attribute-codegen.cs` can be compiled to a tool that collects all Manifest elements and attributes with the API level since when each of them became available. New members are supposed to be added to the existing `(FooBar)Attribute.cs` and `(FooBar)Attribute.Partial.cs` in `src/Mono.Android` and `src/Xamarin.Android.Build.Tasks` respectively.
+See [`build-tools/manifest-attribute-codegen/README.md`](../../build-tools/manifest-attribute-codegen/README.md)
+for details.
 
-Note that there are documented and undocumented XML nodes, and we don't have to deal with undocumented ones.
+Note that there are documented and undocumented XML nodes, and we don't have to
+deal with undocumented ones.
 
 Android P introduced no documented XML artifact.
 
-7) Update Android Tooling Versions
+### Update Android Tooling Versions
+
+[`Xamarin.Android.Common.props.in`](../../src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.props.in)
+contains multiple MSBuild properties which provide default versions for various Android SDK packages.
+These properties in turn come from properties defined within
+[`Configuration.props`](../../Configuration.props).
 
-These sre located in [Xamarin.Android.Common.props.in](../../src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.props.in). The following MSBuild properties need to be updated to ensure 
-the latest tool versions are being used.
+  * `$(AndroidSdkBuildToolsVersion)`: Android SDK `build-tools` version.
+    Defaults to `$(XABuildToolsFolder)` within `Configuration.props`.
+  * `$(AndroidSdkPlatformToolsVersion)`:  Android SDK `platform-tools` version
+    Defaults to `$(XAPlatformToolsVersion)` within `Configuration.props`.
 
-`AndroidSdkBuildToolsVersion`
-`AndroidSdkPlatformToolsVersion`
-`AndroidSdkToolsVersion`
+The major version should generally match the new API level. For Android P this will be 28.x.x . If a version which exactly matches the API Level is not available then the latest version should be used.
 
-The major version should match the new API level. For Android P this will be 28.x.x . If a version which exactly matches the API Level is not available then the latest version should be used.
+A separate PR should be created which bumps the values within `Configuration.props`
+to ensure that all unit tests pass.
 
 ## Bindings Finalization
 

build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/CheckApiCompatibility.cs

@@ -266,7 +266,7 @@ void dataReceived (object sender, DataReceivedEventArgs args)
 					LogError ($"CheckApiCompatibility found nonacceptable Api breakages for ApiLevel: {ApiLevel}.{Environment.NewLine}{string.Join (Environment.NewLine, lines)}");
 					ReportMissingLines (acceptableIssuesFile.FullName, lines);
 
-					var missingItems = CodeGenDiff.GenerateMissingItems (CodeGenPath, contractAssembly.FullName, implementationAssembly.FullName);
+					var missingItems = CodeGenDiff.GenerateMissingItems (CodeGenPath, contractAssembly.FullName, implementationAssembly.FullName, JdkInfo.CreateTaskLogger (this));
 					if (missingItems.Any ()) {
 						Log.LogMessage (MessageImportance.High, $"{Environment.NewLine}*** CodeGen missing items***{Environment.NewLine}");
 						var indent = 0;

build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/CodeGenDiff.cs

@@ -8,10 +8,10 @@ namespace Xamarin.Android.Tools.BootstrapTasks
 {
 	public sealed class CodeGenDiff
 	{
-		public static List<string> GenerateMissingItems (string codeGenPath, string contractAssembly, string implementationAssembly)
+		public static List<string> GenerateMissingItems (string codeGenPath, string contractAssembly, string implementationAssembly, Action<TraceLevel, string> logger)
 		{
-			var contract = GenerateObjectDescription (codeGenPath, contractAssembly);
-			var implementation = GenerateObjectDescription (codeGenPath, implementationAssembly);
+			var contract = GenerateObjectDescription (codeGenPath, contractAssembly, logger);
+			var implementation = GenerateObjectDescription (codeGenPath, implementationAssembly, logger);
 
 			return Diff (contract, implementation);
 		}
@@ -55,7 +55,7 @@ static List<string> Diff (ObjectDescription contract, ObjectDescription implemen
 			return missingItems;
 		}
 
-		static ObjectDescription GenerateObjectDescription (string codeGenPath, string assembly)
+		static ObjectDescription GenerateObjectDescription (string codeGenPath, string assembly, Action<TraceLevel, string> logger)
 		{
 			ObjectDescription currentObject = new ObjectDescription () { Item = "root" };
 			var objectStack = new Stack<ObjectDescription> ();
@@ -74,6 +74,8 @@ static ObjectDescription GenerateObjectDescription (string codeGenPath, string a
 
 				genApiProcess.StartInfo.Arguments += $"\"{assembly}\"";
 
+				logger (TraceLevel.Verbose, $"Executing: `\"{genApiProcess.StartInfo.FileName}\" {genApiProcess.StartInfo.Arguments}`");
+
 				genApiProcess.StartInfo.UseShellExecute = false;
 				genApiProcess.StartInfo.CreateNoWindow = true;
 				genApiProcess.StartInfo.RedirectStandardOutput = true;
@@ -110,7 +112,12 @@ void dataReceived (object sender, DataReceivedEventArgs args)
 
 					if (content.StartsWith ("}", StringComparison.OrdinalIgnoreCase)) {
 						currentObject.InternalCounter--;
-						System.Diagnostics.Debug.Assert (currentObject.InternalCounter >= 0);
+						if (currentObject.InternalCounter < 0) {
+							logger (TraceLevel.Error, $"Internal Error! currentObject.InternalCounter is {currentObject.InternalCounter}; must be >= 0! " +
+								$"currentObject.Item=`{currentObject.Item}`");
+							currentObject.InternalCounter = 0;
+						}
+
 						if (currentObject.InternalCounter == 0) {
 							objectStack.Pop ();
 							if (objectStack.Count > 0) {
@@ -121,7 +128,7 @@ void dataReceived (object sender, DataReceivedEventArgs args)
 						return;
 					}
 
-					if (content.StartsWith ("namespace ", StringComparison.Ordinal) || content.IndexOf (" interface ", StringComparison.Ordinal) != -1 || content.IndexOf (" class ", StringComparison.Ordinal) != -1 || content.IndexOf (" partial struct ", StringComparison.Ordinal) != -1 || content.IndexOf (" enum ", StringComparison.Ordinal) != -1) {
+					if (content.StartsWith ("namespace ", StringComparison.Ordinal) || LineBeginsType (content)) {
 						if (string.IsNullOrWhiteSpace (currentObject.Item)) {
 							currentObject.Item = content;
 						} else {
@@ -163,6 +170,17 @@ void dataReceived (object sender, DataReceivedEventArgs args)
 			return currentObject;
 		}
 
+		static bool LineBeginsType (string content)
+		{
+			if (content.IndexOf (" partial interface ", StringComparison.Ordinal) != -1
+					|| content.IndexOf (" partial class ", StringComparison.Ordinal) != -1
+					|| content.IndexOf (" partial struct ", StringComparison.Ordinal) != -1
+					|| content.IndexOf (" enum ", StringComparison.Ordinal) != -1) {
+				return true;
+			}
+			return false;
+		}
+
 		class ObjectDescription
 		{
 			public string Item;