[Hoststubgen] Allow substitution methods to have...

... smaller visibility.

Fix: 340893548
Test: $ANDROID_BUILD_TOP/frameworks/base/ravenwood/scripts/run-ravenwood-tests.sh
Flag: EXEMPT change affects only Ravenwood (hostside) tests
Change-Id: Ib0b781754f55b759eb76c432f2f5d23f326b0e8c

core/java/com/android/internal/ravenwood/RavenwoodEnvironment.java

@@ -49,7 +49,7 @@ public class RavenwoodEnvironment {
         return false;
     }
 
-    public boolean isRunningOnRavenwood$ravenwood() {
+    private boolean isRunningOnRavenwood$ravenwood() {
         return true;
     }
 }

tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/asm/AsmUtils.kt

@@ -290,6 +290,16 @@ fun FieldNode.getVisibility(): Visibility {
     return Visibility.fromAccess(this.access)
 }
 
+/** Return the [access] flags without the visibility */
+fun clearVisibility(access: Int): Int {
+    return access and (Opcodes.ACC_PUBLIC or Opcodes.ACC_PROTECTED or Opcodes.ACC_PRIVATE).inv()
+}
+
+/** Return the visibility part of the [access] flags */
+fun getVisibility(access: Int): Int {
+    return access and (Opcodes.ACC_PUBLIC or Opcodes.ACC_PROTECTED or Opcodes.ACC_PRIVATE)
+}
+
 
 /*
 

tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/visitors/BaseAdapter.kt

@@ -195,6 +195,8 @@ abstract class BaseAdapter (
                 return null
             }
 
+            var newAccess = access
+
             // Maybe rename the method.
             val newName: String
             val renameTo = filter.getRenameTo(currentClassName, name, descriptor)
@@ -205,8 +207,9 @@ abstract class BaseAdapter (
                 // (the one with the @substitute/replace annotation).
                 // `name` is the name of the method we're currently visiting, so it's usually a
                 // "...$ravewnwood" name.
-                if (!checkSubstitutionMethodCompatibility(
-                        classes, currentClassName, newName, name, descriptor, options.errors)) {
+                newAccess = checkSubstitutionMethodCompatibility(
+                        classes, currentClassName, newName, name, descriptor, options.errors)
+                if (newAccess == NOT_COMPATIBLE) {
                     return null
                 }
 
@@ -221,7 +224,7 @@ abstract class BaseAdapter (
             // But note, we only use it when calling the super's method,
             // but not for visitMethodInner(), because when subclass wants to change access,
             // it can do so inside visitMethodInner().
-            val newAccess = updateAccessFlags(access, name, descriptor)
+            newAccess = updateAccessFlags(newAccess, name, descriptor)
 
             val ret = visitMethodInner(access, newName, descriptor, signature, exceptions, policy,
                 renameTo != null,
@@ -303,4 +306,4 @@ abstract class BaseAdapter (
             return ret
         }
     }
-}
\ No newline at end of file
+}

tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/visitors/Helper.kt

@@ -17,12 +17,19 @@ package com.android.hoststubgen.visitors
 
 import com.android.hoststubgen.HostStubGenErrors
 import com.android.hoststubgen.asm.ClassNodes
+import com.android.hoststubgen.asm.clearVisibility
 import com.android.hoststubgen.asm.getVisibility
 import com.android.hoststubgen.asm.isStatic
 
+const val NOT_COMPATIBLE: Int = -1
+
 /**
  * Make sure substitution from and to methods have matching definition.
- * (static-ness, visibility.)
+ * (static-ness, etc)
+ *
+ * If the methods are compatible, return the "merged" [access] of the new method.
+ *
+ * If they are not compatible, returns [NOT_COMPATIBLE]
  */
 fun checkSubstitutionMethodCompatibility(
     classes: ClassNodes,
@@ -31,33 +38,31 @@ fun checkSubstitutionMethodCompatibility(
     toMethodName: String, // the one with either a "_host" or "$ravenwood" prefix. (typically)
     descriptor: String,
     errors: HostStubGenErrors,
-): Boolean {
+): Int {
     val from = classes.findMethod(className, fromMethodName, descriptor)
     if (from == null) {
         errors.onErrorFound(
-            "Substitution-from method not found: $className.$fromMethodName$descriptor")
-        return false
+            "Substitution-from method not found: $className.$fromMethodName$descriptor"
+        )
+        return NOT_COMPATIBLE
     }
     val to = classes.findMethod(className, toMethodName, descriptor)
     if (to == null) {
         // This shouldn't happen, because the visitor visited this method...
         errors.onErrorFound(
-            "Substitution-to method not found: $className.$toMethodName$descriptor")
-        return false
+            "Substitution-to method not found: $className.$toMethodName$descriptor"
+        )
+        return NOT_COMPATIBLE
     }
 
     if (from.isStatic() != to.isStatic()) {
         errors.onErrorFound(
             "Substitution method must have matching static-ness: " +
-                    "$className.$fromMethodName$descriptor")
-        return false
-    }
-    if (from.getVisibility().ordinal > to.getVisibility().ordinal) {
-        errors.onErrorFound(
-            "Substitution method cannot have smaller visibility than original: " +
-                    "$className.$fromMethodName$descriptor")
-        return false
+                    "$className.$fromMethodName$descriptor"
+        )
+        return NOT_COMPATIBLE
     }
 
-    return true
+    // Return the substitution's access flag but with the original method's visibility.
+    return clearVisibility (to.access) or getVisibility(from.access)
 }

tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/01-hoststubgen-test-tiny-framework-orig-dump.txt

@@ -644,9 +644,9 @@ public class com.android.hoststubgen.test.tinyframework.TinyFrameworkClassAnnota
           suffix="_host"
         )
 
-  public static int nativeAddThree_host(int);
+  private static int nativeAddThree_host(int);
     descriptor: (I)I
-    flags: (0x0009) ACC_PUBLIC, ACC_STATIC
+    flags: (0x000a) ACC_PRIVATE, ACC_STATIC
     Code:
       stack=2, locals=1, args_size=1
          x: iload_0

tools/hoststubgen/hoststubgen/test-tiny-framework/tiny-framework/src/com/android/hoststubgen/test/tinyframework/TinyFrameworkClassAnnotations.java

@@ -73,7 +73,8 @@ public class TinyFrameworkClassAnnotations {
     @HostSideTestSubstitute(suffix = "_host")
     public static native int nativeAddThree(int value);
 
-    public static int nativeAddThree_host(int value) {
+    // This method is private, but at runtime, it'll inherit the visibility of the original method
+    private static int nativeAddThree_host(int value) {
         return value + 3;
     }
 

tools/hoststubgen/hoststubgen/test/com/android/hoststubgen/visitors/HelperTest.kt

@@ -71,7 +71,7 @@ class HelperTest {
             addClass(cn)
         }
 
-        fun check(from: MethodNode?, to: MethodNode?, expected: Boolean) {
+        fun check(from: MethodNode?, to: MethodNode?, expected: Int) {
             assertThat(checkSubstitutionMethodCompatibility(
                 classes,
                 cn.name,
@@ -82,21 +82,21 @@ class HelperTest {
             )).isEqualTo(expected)
         }
 
-        check(staticPublic, staticPublic, true)
-        check(staticPrivate, staticPrivate, true)
-        check(nonStaticPublic, nonStaticPublic, true)
-        check(nonStaticPProtected, nonStaticPProtected, true)
+        check(staticPublic, staticPublic, Opcodes.ACC_PUBLIC or Opcodes.ACC_STATIC)
+        check(staticPrivate, staticPrivate, Opcodes.ACC_PRIVATE or Opcodes.ACC_STATIC)
+        check(nonStaticPublic, nonStaticPublic, Opcodes.ACC_PUBLIC)
+        check(nonStaticPProtected, nonStaticPProtected, 0)
 
-        check(staticPublic, null, false)
-        check(null, staticPublic, false)
+        check(staticPublic, null, NOT_COMPATIBLE)
+        check(null, staticPublic, NOT_COMPATIBLE)
 
-        check(staticPublic, nonStaticPublic, false)
-        check(nonStaticPublic, staticPublic, false)
+        check(staticPublic, nonStaticPublic, NOT_COMPATIBLE)
+        check(nonStaticPublic, staticPublic, NOT_COMPATIBLE)
 
-        check(staticPublic, staticPrivate, false)
-        check(staticPrivate, staticPublic, true)
+        check(staticPublic, staticPrivate, Opcodes.ACC_PUBLIC or Opcodes.ACC_STATIC)
+        check(staticPrivate, staticPublic, Opcodes.ACC_PRIVATE or Opcodes.ACC_STATIC)
 
-        check(nonStaticPublic, nonStaticPProtected, false)
-        check(nonStaticPProtected, nonStaticPublic, true)
+        check(nonStaticPublic, nonStaticPProtected, Opcodes.ACC_PUBLIC)
+        check(nonStaticPProtected, nonStaticPublic, 0)
     }
 }
\ No newline at end of file