[HSG] Allow class directive without policy

In some cases, we want to add a policy for a member (field/method) in
the policy file for a class that already has an annotation.

Previously, we had to always have some policy in the class line, which
could cause confusion because this will essentially override the
annotation, so even when you change the class annotation, that wouldn't
be picked up.

Now update the parser so that a class directive is allowed to omit
a policy.

Also fix b/388562869. But I still need to ignore certain lines in diff
because the merged golden files are already missing the expected lines.

Flag: EXEMPT host test change only
Bug: 292141694
Bug: 388562869
Test: $ANDROID_BUILD_TOP/frameworks/base/ravenwood/scripts/run-ravenwood-tests.sh -s
Change-Id: Ifa37cd0ff74026bc1ba2c7e8599b159fa0a07412

5 files changed, 67 insertions, 21 deletions

diff --git a/ravenwood/texts/ravenwood-framework-policies.txt b/ravenwood/texts/ravenwood-framework-policies.txt
index 80126df1b8df..26b6fe3d82ad 100644
--- a/ravenwood/texts/ravenwood-framework-policies.txt
+++ b/ravenwood/texts/ravenwood-framework-policies.txt
@@ -52,7 +52,7 @@ class android.content.BroadcastReceiver keep
 class android.content.Context keep
     method <init> ()V keep
     method getSystemService (Ljava/lang/Class;)Ljava/lang/Object; keep
-class android.content.pm.PackageManager keep
+class android.content.pm.PackageManager
     method <init> ()V keep
 class android.text.ClipboardManager keep
     method <init> ()V keep
diff --git a/ravenwood/tools/hoststubgen/scripts/dump-jar b/ravenwood/tools/hoststubgen/scripts/dump-jar
index 998357b70dff..02a6d25378bd 100755
--- a/ravenwood/tools/hoststubgen/scripts/dump-jar
+++ b/ravenwood/tools/hoststubgen/scripts/dump-jar
@@ -89,14 +89,33 @@ filter_output() {
     # - Some other transient lines
     # - Sometimes the javap shows mysterious warnings, so remove them too.
     #
-    # `/PATTERN-1/,/PATTERN-2/{//!d}` is a trick to delete lines between two patterns, without
-    # the start and the end lines.
+    # Most conversion are simple per-line deletion or simple inline replacement with a regex.
+    #
+    # But removing the constant pool is a bit tricky. It looks like this in the output:
+    #---------------------------
+    #Constant pool:
+    #    #1 = Methodref          #31.#88       // java/lang/Object."<init>":()V
+    #    #2 = Class              #89           // java/lang/UnsupportedOperationException
+    #    :
+    #{ // Or something, I'm not sure if it always ends with a "{".
+    #---------------------------
+    # i.e. we want to delete all lines from "Constant pool:" as long as the first character
+    # is a space.
+    #
+    # If we simply use '/^Constant pool:/,/^[^ ]/d', then it'll delete the "Constant pool:"
+    # line and "{" line too, but again the last line might be important, so we don't want to
+    # delete it.
+    #
+    # So we instead, use '/^Constant pool:/,/^[^ ]/{/^ /d}', which mean:
+    # between lines matching '/^Constant pool:/' and '/^[^ ]/', delete lines that start with
+    # a space. (=='/^ /d').
+    #
     sed -e 's/#[0-9][0-9]*/#x/g' \
         -e 's/^\( *\)[0-9][0-9]*:/\1x:/' \
-        -e '/^Constant pool:/,/^[^ ]/{//!d}' \
+        -e '/^Constant pool:/,/^[^ ]/{/^ /d}' \
         -e '/^ *line *[0-9][0-9]*: *[0-9][0-9]*$/d' \
-        -e '/SHA-256 checksum/d' \
-        -e '/Last modified/d' \
+        -e '/^ *SHA-256 checksum/d' \
+        -e '/^ *Last modified/d' \
         -e '/^Classfile jar/d' \
         -e '/\[warning\]/d'
   else
diff --git a/ravenwood/tools/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt b/ravenwood/tools/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt
index be1b6ca93869..c5500831e21a 100644
--- a/ravenwood/tools/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt
+++ b/ravenwood/tools/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt
@@ -413,8 +413,8 @@ class TextFileFilterPolicyParser {
     }
 
     private fun parseClass(fields: Array<String>) {
-        if (fields.size < 3) {
-            throw ParseException("Class ('c') expects 2 fields.")
+        if (fields.size <= 1) {
+            throw ParseException("Class ('c') expects 1 or 2 fields.")
         }
         val className = fields[1]
 
@@ -424,7 +424,9 @@ class TextFileFilterPolicyParser {
         // :aidl, etc?
         val classType = resolveSpecialClass(className)
 
-        if (fields[2].startsWith("!")) {
+        val policyStr = if (fields.size > 2) { fields[2] } else { "" }
+
+        if (policyStr.startsWith("!")) {
             if (classType != SpecialClass.NotSpecial) {
                 // We could support it, but not needed at least for now.
                 throw ParseException(
@@ -432,10 +434,10 @@ class TextFileFilterPolicyParser {
                 )
             }
             // It's a redirection class.
-            val toClass = fields[2].substring(1)
+            val toClass = policyStr.substring(1)
 
             processor.onRedirectionClass(className, toClass)
-        } else if (fields[2].startsWith("~")) {
+        } else if (policyStr.startsWith("~")) {
             if (classType != SpecialClass.NotSpecial) {
                 // We could support it, but not needed at least for now.
                 throw ParseException(
@@ -443,11 +445,23 @@ class TextFileFilterPolicyParser {
                 )
             }
             // It's a class-load hook
-            val callback = fields[2].substring(1)
+            val callback = policyStr.substring(1)
 
             processor.onClassLoadHook(className, callback)
         } else {
-            val policy = parsePolicy(fields[2])
+            // Special case: if it's a class directive with no policy, then it encloses
+            // members, but we don't apply any policy to the class itself.
+            // This is only allowed in a not-special case.
+            if (policyStr == "") {
+                if (classType == SpecialClass.NotSpecial && superClass == null) {
+                    currentClassName = className
+                    processor.onSimpleClassStart(className)
+                    return
+                }
+                throw ParseException("Special class or subclass directive must have a policy")
+            }
+
+            val policy = parsePolicy(policyStr)
             if (!policy.isUsableWithClasses) {
                 throw ParseException("Class can't have policy '$policy'")
             }
diff --git a/ravenwood/tools/hoststubgen/test-tiny-framework/diff-and-update-golden.sh b/ravenwood/tools/hoststubgen/test-tiny-framework/diff-and-update-golden.sh
index 8408a18142eb..23699fd1dba4 100755
--- a/ravenwood/tools/hoststubgen/test-tiny-framework/diff-and-update-golden.sh
+++ b/ravenwood/tools/hoststubgen/test-tiny-framework/diff-and-update-golden.sh
@@ -37,8 +37,7 @@ SCRIPT_NAME="${0##*/}"
 GOLDEN_DIR=${GOLDEN_DIR:-golden-output}
 mkdir -p $GOLDEN_DIR
 
-# TODO(b/388562869) We shouldn't need `--ignore-matching-lines`, but the golden files may not have the "Constant pool:" lines.
-DIFF_CMD=${DIFF_CMD:-diff -u --ignore-blank-lines --ignore-space-change --ignore-matching-lines='^\(Constant.pool:\|{\)$'}
+DIFF_CMD=${DIFF_CMD:-./tiny-framework-dump-test.py run-diff}
 
 update=0
 three_way=0
@@ -63,12 +62,10 @@ done
 shift $(($OPTIND - 1))
 
 # Build the dump files, which are the input of this test.
-run ${BUILD_CMD:=m} dump-jar tiny-framework-dump-test
-
+run ${BUILD_CMD:-m} dump-jar tiny-framework-dump-test
 
 # Get the path to the generate text files. (not the golden files.)
 # We get them from $OUT/module-info.json
-
 files=(
 $(python3 -c '
 import sys
@@ -78,7 +75,7 @@ import json
 with open(sys.argv[1], "r") as f:
     data = json.load(f)
 
-    # Equivalent to: jq -r '.["tiny-framework-dump-test"]["installed"][]'
+    # Equivalent to:    jq -r '.["tiny-framework-dump-test"]["installed"][]'
     for path in data["tiny-framework-dump-test"]["installed"]:
 
       if "golden-output" in path:
diff --git a/ravenwood/tools/hoststubgen/test-tiny-framework/tiny-framework-dump-test.py b/ravenwood/tools/hoststubgen/test-tiny-framework/tiny-framework-dump-test.py
index c35d6d106c8d..761748265726 100755
--- a/ravenwood/tools/hoststubgen/test-tiny-framework/tiny-framework-dump-test.py
+++ b/ravenwood/tools/hoststubgen/test-tiny-framework/tiny-framework-dump-test.py
@@ -28,9 +28,16 @@ GOLDEN_DIRS = [
 
 # Run diff.
 def run_diff(file1, file2):
-    # TODO(b/388562869) We shouldn't need `--ignore-matching-lines`, but the golden files may not have the "Constant pool:" lines.
-    command = ['diff', '-u', '--ignore-blank-lines',
+    command = ['diff', '-u',
+               '--ignore-blank-lines',
                '--ignore-space-change',
+
+               # Ignore the class file version.
+               '--ignore-matching-lines=^ *\(major\|minor\) version:$',
+
+               # We shouldn't need `--ignore-matching-lines`, but somehow
+               # the golden files were generated without these lines for b/388562869,
+               # so let's just ignore them.
                '--ignore-matching-lines=^\(Constant.pool:\|{\)$',
                file1, file2]
     print(' '.join(command))
@@ -85,4 +92,13 @@ class TestWithGoldenOutput(unittest.TestCase):
 
 
 if __name__ == "__main__":
+    args = sys.argv
+
+    # This script is used by diff-and-update-golden.sh too.
+    if len(args) > 1 and args[1] == "run-diff":
+        if run_diff(args[2], args[3]):
+            sys.exit(0)
+        else:
+            sys.exit(1)
+
     unittest.main(verbosity=2)