Query PlatformCompat for targetSdk check in AndroidPackageParsingTestBase

There are prebuilts in the tree and perhaps APKs in general
which don't conform to the manifest checks added in R, so the code
that parses the APK to verify v1 vs v2 needs to actually query
PlatformCompat to do the proper targetSdkVersion check and allow
the APK through if it's not updated yet.

A related change to default enabled inside ParsingPackageImpl to
true was also made, as if any malformed APK was allowed through
without an <application> tag, it would never hit the code
where enabled gets assigned to default true.

Any further errors spit out by this test are critical and need to
updated so that their APKs can be installed on R.

Bug: 153058196

Test: manual verify error behavior with broken APK
Test: atest com.android.server.pm.parsing

Change-Id: I2b117f2098d8bd62b92921178a098e838b55b06d

3 files changed, 80 insertions, 26 deletions

diff --git a/core/java/android/content/pm/parsing/ParsingPackageImpl.java b/core/java/android/content/pm/parsing/ParsingPackageImpl.java
index 894ad5584922..be1817d09155 100644
--- a/core/java/android/content/pm/parsing/ParsingPackageImpl.java
+++ b/core/java/android/content/pm/parsing/ParsingPackageImpl.java
@@ -322,7 +322,12 @@ public class ParsingPackageImpl implements ParsingPackage, Parcelable {
     private String className;
     private int compatibleWidthLimitDp;
     private int descriptionRes;
-    private boolean enabled;
+
+    // Usually there's code to set this to true during parsing, but it's possible to install an APK
+    // targeting <R that doesn't contain an <application> tag. That code would be skipped and never
+    // assign this, so initialize this to true for those cases.
+    private boolean enabled = true;
+
     private boolean crossProfile;
     private int fullBackupContent;
     private int iconRes;
diff --git a/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingEquivalenceTest.kt b/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingEquivalenceTest.kt
index 191c038c3052..5412bb5106ff 100644
--- a/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingEquivalenceTest.kt
+++ b/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingEquivalenceTest.kt
@@ -18,7 +18,9 @@ package com.android.server.pm.parsing
 
 import android.content.pm.PackageManager
 import android.platform.test.annotations.Presubmit
+import com.google.common.truth.Expect
 import com.google.common.truth.Truth.assertWithMessage
+import org.junit.Rule
 import org.junit.Test
 
 /**
@@ -28,6 +30,9 @@ import org.junit.Test
 @Presubmit
 class AndroidPackageParsingEquivalenceTest : AndroidPackageParsingTestBase() {
 
+    @get:Rule
+    val expect = Expect.create()
+
     @Test
     fun applicationInfoEquality() {
         val flags = PackageManager.GET_META_DATA or PackageManager.GET_SHARED_LIBRARY_FILES
@@ -41,7 +46,8 @@ class AndroidPackageParsingEquivalenceTest : AndroidPackageParsingTestBase() {
             } else {
                 "$firstName | $secondName"
             }
-            assertWithMessage(packageName).that(it.first?.dumpToString())
+            expect.withMessage("${it.first?.sourceDir} $packageName")
+                    .that(it.first?.dumpToString())
                     .isEqualTo(it.second?.dumpToString())
         }
     }
@@ -71,7 +77,8 @@ class AndroidPackageParsingEquivalenceTest : AndroidPackageParsingTestBase() {
             } else {
                 "$firstName | $secondName"
             }
-            assertWithMessage(packageName).that(it.first?.dumpToString())
+            expect.withMessage("${it.first?.applicationInfo?.sourceDir} $packageName")
+                    .that(it.first?.dumpToString())
                     .isEqualTo(it.second?.dumpToString())
         }
     }
diff --git a/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt b/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt
index 19bf9b673f8b..7b1b2d2f5c2b 100644
--- a/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt
+++ b/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt
@@ -29,14 +29,17 @@ import android.content.pm.PermissionInfo
 import android.content.pm.ProviderInfo
 import android.os.Debug
 import android.os.Environment
+import android.os.ServiceManager
 import android.util.SparseArray
 import androidx.test.platform.app.InstrumentationRegistry
+import com.android.internal.compat.IPlatformCompat
 import com.android.server.pm.PackageManagerService
 import com.android.server.pm.PackageSetting
 import com.android.server.pm.parsing.pkg.AndroidPackage
 import com.android.server.pm.pkg.PackageStateUnserialized
 import com.android.server.testutils.mockThrowOnUnmocked
 import com.android.server.testutils.whenever
+import org.junit.After
 import org.junit.BeforeClass
 import org.mockito.Mockito
 import org.mockito.Mockito.anyInt
@@ -59,7 +62,27 @@ open class AndroidPackageParsingTestBase {
             setCallback { false /* hasFeature */ }
         }
 
-        protected val packageParser2 = TestPackageParser2()
+        private val platformCompat = IPlatformCompat.Stub
+                .asInterface(ServiceManager.getService(Context.PLATFORM_COMPAT_SERVICE))
+
+        protected val packageParser2 = PackageParser2(null /* separateProcesses */,
+                false /* onlyCoreApps */, context.resources.displayMetrics, null /* cacheDir */,
+                object : PackageParser2.Callback() {
+                    override fun isChangeEnabled(
+                        changeId: Long,
+                        appInfo: ApplicationInfo
+                    ): Boolean {
+                        // This test queries PlatformCompat because prebuilts in the tree
+                        // may not be updated to be compliant with the latest enforcement checks.
+                        return platformCompat.isChangeEnabled(changeId, appInfo)
+                    }
+
+                    // Assume the device doesn't support anything. This will affect permission
+                    // parsing and will force <uses-permission/> declarations to include all
+                    // requiredNotFeature permissions and exclude all requiredFeature permissions.
+                    // This mirrors the old behavior.
+                    override fun hasFeature(feature: String) = false
+                })
 
         /**
          * It would be difficult to mock all possibilities, so just use the APKs on device.
@@ -91,35 +114,31 @@ open class AndroidPackageParsingTestBase {
 
         lateinit var newPackages: List<AndroidPackage>
 
-        var failureInBeforeClass: Throwable? = null
+        private val thrownInSetUp = mutableListOf<Throwable>()
 
         @Suppress("ConstantConditionIf")
         @JvmStatic
         @BeforeClass
         fun setUpPackages() {
-            failureInBeforeClass = null
-            try {
-                this.oldPackages = apks.map {
+            this.oldPackages = apks.mapNotNull {
+                tryOrNull {
                     packageParser.parsePackage(it, PackageParser.PARSE_IS_SYSTEM_DIR, false)
                 }
+            }
 
-                this.newPackages = apks.map {
+            this.newPackages = apks.mapNotNull {
+                tryOrNull {
                     packageParser2.parsePackage(it, PackageParser.PARSE_IS_SYSTEM_DIR, false)
                 }
+            }
 
-                if (DUMP_HPROF_TO_EXTERNAL) {
-                    System.gc()
-                    Environment.getExternalStorageDirectory()
-                            .resolve(
-                                    "${AndroidPackageParsingTestBase::class.java.simpleName}.hprof")
-                            .absolutePath
-                            .run(Debug::dumpHprofData)
-                }
-            } catch (t: Throwable) {
-                // If we crash here we cause a tool failure (because we don't run any of the tests
-                // in the subclasses, leading to a difference between expected and actual test
-                // result counts).
-                failureInBeforeClass = t
+            if (DUMP_HPROF_TO_EXTERNAL) {
+                System.gc()
+                Environment.getExternalStorageDirectory()
+                        .resolve(
+                                "${AndroidPackageParsingTestBase::class.java.simpleName}.hprof")
+                        .absolutePath
+                        .run(Debug::dumpHprofData)
             }
         }
 
@@ -146,13 +165,36 @@ open class AndroidPackageParsingTestBase {
             this.pkg = aPkg
             whenever(pkgState) { PackageStateUnserialized() }
         }
+
+        private fun <T> tryOrNull(block: () -> T) = try {
+            block()
+        } catch (t: Throwable) {
+            thrownInSetUp.add(t)
+            null
+        }
     }
 
-    @org.junit.Before
+    @After
     fun verifySetUpPackages() {
-        failureInBeforeClass?.let {
-            throw AssertionError("setUpPackages failed:", it)
-        }
+        if (thrownInSetUp.isEmpty()) return
+        val exception = AssertionError("setUpPackages failed with ${thrownInSetUp.size} errors:\n" +
+                thrownInSetUp.joinToString(separator = "\n") { it.message.orEmpty() })
+
+        /*
+            Testing infrastructure doesn't currently support errors thrown in @AfterClass,
+            so instead it's thrown here. But to avoid throwing a massive repeated stack for every
+            test method, only throw on the first method run in the class, clearing the list so that
+            subsequent methods can run without failing. Doing this in @After lets true method
+            failures propagate, as those should throw before this does.
+
+            This will cause the failure to be attached to a different method depending on run order,
+            which could make comparisons difficult. So if a failure points here, it's worth
+            checking failures for all methods in all subclasses.
+
+            TODO: When infrastructure supports @AfterClass errors, move this
+        */
+        thrownInSetUp.clear()
+        throw exception
     }
 
     // The following methods dump an exact set of fields from the object to compare, because