From ac9e76c8d63c9bb320aadc6357091238d240166a Mon Sep 17 00:00:00 2001 From: Winson Date: Tue, 23 Feb 2021 09:22:31 -0800 Subject: [PATCH] Migrate DomainVerificationUtils to no log ChangeId Uses the isChangeEnabledInternalNoLogging variant, which skips the caller permission check, which improves performance. Also removes the need to clear calling identity. This uses a mocked ApplicationInfo as it can be called during package update, which means the PM lock cannot be taken. This, and in all other cases, the method is being called as part of a service side check, post feature/permission app visibility enforcement, so it should be safe to skip permission checks. This isn't enforced, but since DomainVerificationUtils#isChangeEnabled is only visible inside the DVS package, it should be fine. Bug: 159952358 Test: atest com.android.server.pm.verify.domain Change-Id: I9c54e8653d843cfb67fb9d6e12349cf06de90fce --- .../server/pm/verify/domain/DomainVerificationUtils.java | 5 +---- .../test/verify/domain/DomainVerificationCollectorTest.kt | 7 ++++++- .../test/verify/domain/DomainVerificationEnforcerTest.kt | 7 +------ .../test/verify/domain/DomainVerificationManagerApiTest.kt | 2 +- .../pm/test/verify/domain/DomainVerificationPackageTest.kt | 3 ++- .../domain/DomainVerificationSettingsMutationTest.kt | 2 +- .../domain/DomainVerificationUserSelectionOverrideTest.kt | 2 +- 7 files changed, 13 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationUtils.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationUtils.java index cb3b5c9db7e7c..44ff3eb4b9423 100644 --- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationUtils.java +++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationUtils.java @@ -22,7 +22,6 @@ import android.content.Intent; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; -import android.os.Binder; import com.android.internal.util.CollectionUtils; import com.android.server.compat.PlatformCompat; @@ -77,9 +76,7 @@ public final class DomainVerificationUtils { static boolean isChangeEnabled(PlatformCompat platformCompat, AndroidPackage pkg, long changeId) { - //noinspection ConstantConditions - return Binder.withCleanCallingIdentity( - () -> platformCompat.isChangeEnabled(changeId, buildMockAppInfo(pkg))); + return platformCompat.isChangeEnabledInternalNoLogging(changeId, buildMockAppInfo(pkg)); } /** diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCollectorTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCollectorTest.kt index d5eda203e42f6..dce853afc7632 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCollectorTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationCollectorTest.kt @@ -41,7 +41,12 @@ class DomainVerificationCollectorTest { } private val platformCompat: PlatformCompat = mockThrowOnUnmocked { - whenever(isChangeEnabled(eq(DomainVerificationCollector.RESTRICT_DOMAINS), any())) { + whenever( + isChangeEnabledInternalNoLogging( + eq(DomainVerificationCollector.RESTRICT_DOMAINS), + any() + ) + ) { (arguments[1] as ApplicationInfo).targetSdkVersion >= Build.VERSION_CODES.S } } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt index 7e25901301aaa..1b0a305b5bddb 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationEnforcerTest.kt @@ -114,12 +114,7 @@ class DomainVerificationEnforcerTest { it, mockThrowOnUnmocked { whenever(linkedApps) { ArraySet() } }, mockThrowOnUnmocked { - whenever( - isChangeEnabled( - anyLong(), - any() - ) - ) { true } + whenever(isChangeEnabledInternalNoLogging(anyLong(), any())) { true } }).apply { setConnection(connection) } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt index 0e74b65d25d57..ef79b08aa1c5f 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt @@ -310,7 +310,7 @@ class DomainVerificationManagerApiTest { }, mockThrowOnUnmocked { whenever(linkedApps) { ArraySet() } }, mockThrowOnUnmocked { - whenever(isChangeEnabled(anyLong(), any())) { true } + whenever(isChangeEnabledInternalNoLogging(anyLong(), any())) { true } }).apply { setConnection(mockThrowOnUnmocked { whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt index fe3672d06bc03..0ce16e6b6af28 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationPackageTest.kt @@ -43,6 +43,7 @@ import org.junit.Test import org.mockito.ArgumentMatchers import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.anyLong import org.mockito.ArgumentMatchers.anyString import java.util.UUID @@ -366,7 +367,7 @@ class DomainVerificationPackageTest { }, mockThrowOnUnmocked { whenever(linkedApps) { ArraySet() } }, mockThrowOnUnmocked { - whenever(isChangeEnabled(ArgumentMatchers.anyLong(), any())) { true } + whenever(isChangeEnabledInternalNoLogging(anyLong(), any())) { true } }).apply { setConnection(mockThrowOnUnmocked { whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt index 377bae15e2d52..b7c69226ded20 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationSettingsMutationTest.kt @@ -98,7 +98,7 @@ class DomainVerificationSettingsMutationTest { context, mockThrowOnUnmocked { whenever(linkedApps) { ArraySet() } }, mockThrowOnUnmocked { - whenever(isChangeEnabled(anyLong(),any())) { true } + whenever(isChangeEnabledInternalNoLogging(anyLong(), any())) { true } }).apply { setConnection(connection) } diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt index 44c1b8f3fbb95..54648ab674229 100644 --- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt +++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationUserSelectionOverrideTest.kt @@ -71,7 +71,7 @@ class DomainVerificationUserStateOverrideTest { }, mockThrowOnUnmocked { whenever(linkedApps) { ArraySet() } }, mockThrowOnUnmocked { - whenever(isChangeEnabled(anyLong(), any())) { true } + whenever(isChangeEnabledInternalNoLogging(anyLong(), any())) { true } }).apply { setConnection(mockThrowOnUnmocked { whenever(filterAppAccess(anyString(), anyInt(), anyInt())) { false }