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
diff --git a/core/java/android/content/pm/parsing/ParsingPackageImpl.java b/core/java/android/content/pm/parsing/ParsingPackageImpl.java
index 894ad55..be1817d 100644
--- a/core/java/android/content/pm/parsing/ParsingPackageImpl.java
+++ b/core/java/android/content/pm/parsing/ParsingPackageImpl.java
@@ -322,7 +322,12 @@
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 191c038..5412bb5 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 @@
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 @@
@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 @@
} 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 @@
} 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 19bf9b6..7b1b2d2 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.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 @@
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 @@
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 @@
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