Allow nested calls to callWithShellPermissionIdentity.
This is quite confusing that permissions are not merged when calling
callWithShellPermissionIdentity in a nested block.
Bug: 221401739
Test: atest CtsSafetyCenterTestCases
Change-Id: I8bb577e024ce49098fa3bc418ab52cb28037676f
diff --git a/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterActivityTest.kt b/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterActivityTest.kt
index effe270..94abe35 100644
--- a/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterActivityTest.kt
+++ b/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterActivityTest.kt
@@ -35,6 +35,8 @@
import android.safetycenter.cts.testing.SafetySourceCtsData
import android.support.test.uiautomator.By
import android.support.test.uiautomator.BySelector
+import android.support.test.uiautomator.StaleObjectException
+import android.support.test.uiautomator.UiDevice
import android.support.test.uiautomator.UiObject2
import android.util.Log
import androidx.test.core.app.ApplicationProvider.getApplicationContext
@@ -81,11 +83,7 @@
return
}
safetyCenterCtsHelper.reset()
- }
-
- @After
- fun resetDeviceState() {
- resetDeviceRotation()
+ getUiDevice().resetRotation()
}
@Test
@@ -251,7 +249,7 @@
assertIssueDisplayed(safetySourceCtsData.informationIssue)
// Device rotation to trigger usage of savedinstancestate via config update
- rotateDevice()
+ uiDevice.rotate()
// Verify cards remain expanded
waitTextNotDisplayed("See all alerts")
@@ -310,7 +308,7 @@
null) {
return
}
- } catch (e: android.support.test.uiautomator.StaleObjectException) {
+ } catch (e: StaleObjectException) {
Log.d(
TAG,
"StaleObjectException while calling waitTextNotDisplayed, will retry " +
@@ -331,28 +329,27 @@
waitFindObject(By.text("See all alerts")).click()
}
- private fun rotateDevice() {
- val uiDevice = getUiDevice()
- if (uiDevice.isNaturalOrientation) {
- uiDevice.setOrientationLeft()
- } else {
- uiDevice.setOrientationNatural()
- }
- uiDevice.waitForIdle()
- }
-
- private fun resetDeviceRotation() {
- val uiDevice = getUiDevice()
- if (!uiDevice.isNaturalOrientation) {
- uiDevice.setOrientationNatural()
- uiDevice.waitForIdle()
- }
- }
-
companion object {
private val TAG = SafetyCenterActivityTest::class.java.simpleName
private val NOT_DISPLAYED_TIMEOUT = Duration.ofSeconds(20)
private val NOT_DISPLAYED_CHECK_INTERVAL = Duration.ofMillis(100)
private val FIND_TEXT_TIMEOUT = Duration.ofSeconds(25)
+
+ private fun UiDevice.rotate() {
+ if (isNaturalOrientation) {
+ setOrientationLeft()
+ } else {
+ setOrientationNatural()
+ }
+ waitForIdle()
+ }
+
+ private fun UiDevice.resetRotation() {
+ if (isNaturalOrientation) {
+ return
+ }
+ setOrientationNatural()
+ waitForIdle()
+ }
}
}
diff --git a/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterManagerTest.kt b/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterManagerTest.kt
index 8981f71..cd67451 100644
--- a/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterManagerTest.kt
+++ b/tests/cts/safetycenter/src/android/safetycenter/cts/SafetyCenterManagerTest.kt
@@ -16,7 +16,6 @@
package android.safetycenter.cts
-import android.Manifest.permission.MANAGE_SAFETY_CENTER
import android.app.PendingIntent
import android.content.Context
import android.content.Intent
@@ -116,7 +115,6 @@
import androidx.test.core.app.ApplicationProvider.getApplicationContext
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SdkSuppress
-import com.android.compatibility.common.util.SystemUtil.callWithShellPermissionIdentity
import com.google.common.truth.Truth.assertThat
import com.google.common.util.concurrent.MoreExecutors.directExecutor
import kotlin.test.assertFailsWith
@@ -1369,29 +1367,22 @@
}
@Test
- // Permission is held for the entire test to avoid a racy scenario where the shell identity is
- // dropped while it's being acquired on another thread.
fun addOnSafetyCenterDataChangedListener_oneShot_doesntDeadlock() {
- callWithShellPermissionIdentity(
- {
- val listener = SafetyCenterCtsListener()
- val oneShotListener =
- object : OnSafetyCenterDataChangedListener {
- override fun onSafetyCenterDataChanged(safetyCenterData: SafetyCenterData) {
- safetyCenterManager.removeOnSafetyCenterDataChangedListener(this)
- listener.onSafetyCenterDataChanged(safetyCenterData)
- }
- }
- safetyCenterManager.addOnSafetyCenterDataChangedListener(
- directExecutor(), oneShotListener)
+ val listener = SafetyCenterCtsListener()
+ val oneShotListener =
+ object : OnSafetyCenterDataChangedListener {
+ override fun onSafetyCenterDataChanged(safetyCenterData: SafetyCenterData) {
+ safetyCenterManager.removeOnSafetyCenterDataChangedListenerWithPermission(this)
+ listener.onSafetyCenterDataChanged(safetyCenterData)
+ }
+ }
+ safetyCenterManager.addOnSafetyCenterDataChangedListenerWithPermission(
+ directExecutor(), oneShotListener)
- // Check that we don't deadlock when using a one-shot listener: this is because
- // adding the listener could call the listener while holding a lock on the binder
- // thread-pool; causing a deadlock when attempting to call the `SafetyCenterManager`
- // from that listener.
- listener.receiveSafetyCenterData()
- },
- MANAGE_SAFETY_CENTER)
+ // Check that we don't deadlock when using a one-shot listener. This is because adding the
+ // listener could call it while holding a lock; which would cause a deadlock if the listener
+ // wasn't oneway.
+ listener.receiveSafetyCenterData()
}
@Test
diff --git a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterApisWithShellPermissions.kt b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterApisWithShellPermissions.kt
index fe10928..d72e5da 100644
--- a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterApisWithShellPermissions.kt
+++ b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterApisWithShellPermissions.kt
@@ -26,7 +26,7 @@
import android.safetycenter.SafetySourceData
import android.safetycenter.SafetySourceErrorDetails
import android.safetycenter.config.SafetyCenterConfig
-import com.android.compatibility.common.util.SystemUtil.callWithShellPermissionIdentity
+import android.safetycenter.cts.testing.ShellPermissions.callWithShellPermissionIdentity
import java.util.concurrent.Executor
/**
diff --git a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterEnabledChangedReceiver.kt b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterEnabledChangedReceiver.kt
index da5a447..5172bfa 100644
--- a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterEnabledChangedReceiver.kt
+++ b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterEnabledChangedReceiver.kt
@@ -17,7 +17,6 @@
package android.safetycenter.cts.testing
import android.Manifest.permission.READ_SAFETY_CENTER_STATUS
-import android.Manifest.permission.WRITE_DEVICE_CONFIG
import android.content.BroadcastReceiver
import android.content.Context
import android.content.Intent
@@ -25,7 +24,7 @@
import android.safetycenter.SafetyCenterManager.ACTION_SAFETY_CENTER_ENABLED_CHANGED
import android.safetycenter.cts.testing.Coroutines.TIMEOUT_LONG
import android.safetycenter.cts.testing.Coroutines.runBlockingWithTimeout
-import com.android.compatibility.common.util.SystemUtil.callWithShellPermissionIdentity
+import android.safetycenter.cts.testing.ShellPermissions.callWithShellPermissionIdentity
import java.time.Duration
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.channels.Channel.Factory.UNLIMITED
@@ -60,11 +59,10 @@
) =
callWithShellPermissionIdentity(
{
- SafetyCenterFlags.setSafetyCenterEnabledWithoutPermission(value)
+ SafetyCenterFlags.isEnabled = value
receiveSafetyCenterEnabledChanged(timeout)
},
- READ_SAFETY_CENTER_STATUS,
- WRITE_DEVICE_CONFIG)
+ READ_SAFETY_CENTER_STATUS)
fun reset() {
safetyCenterEnabledChangedChannel.cancel()
diff --git a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterFlags.kt b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterFlags.kt
index 6161e8b..84e06d0 100644
--- a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterFlags.kt
+++ b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetyCenterFlags.kt
@@ -23,7 +23,7 @@
import android.provider.DeviceConfig
import android.provider.DeviceConfig.NAMESPACE_PRIVACY
import android.provider.DeviceConfig.Properties
-import com.android.compatibility.common.util.SystemUtil.callWithShellPermissionIdentity
+import android.safetycenter.cts.testing.ShellPermissions.callWithShellPermissionIdentity
/** A class that facilitates working with Safety Center flags. */
// TODO(b/219553295): Add timeout flags.
@@ -48,27 +48,19 @@
READ_DEVICE_CONFIG)
set(value) {
callWithShellPermissionIdentity(
- { setSafetyCenterEnabledWithoutPermission(value) }, WRITE_DEVICE_CONFIG)
+ {
+ val valueWasSet =
+ DeviceConfig.setProperty(
+ NAMESPACE_PRIVACY,
+ PROPERTY_SAFETY_CENTER_ENABLED,
+ /* value = */ value.toString(),
+ /* makeDefault = */ false)
+ require(valueWasSet) { "Could not set Safety Center flag value to: $value" }
+ },
+ WRITE_DEVICE_CONFIG)
}
/**
- * Sets the Safety Center device config flag to the given boolean [value], but without holding
- * the [WRITE_DEVICE_CONFIG] permission.
- *
- * [callWithShellPermissionIdentity] mutates a global state, so it is not possible to modify
- * [isEnabled] within another call to [callWithShellPermissionIdentity].
- */
- fun setSafetyCenterEnabledWithoutPermission(value: Boolean) {
- val valueWasSet =
- DeviceConfig.setProperty(
- NAMESPACE_PRIVACY,
- PROPERTY_SAFETY_CENTER_ENABLED,
- /* value = */ value.toString(),
- /* makeDefault = */ false)
- require(valueWasSet) { "Could not set Safety Center flag value to: $value" }
- }
-
- /**
* Returns a snapshot of all the Safety Center flags.
*
* This snapshot is only taken once and cached afterwards. This must be called at least once
diff --git a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetySourceReceiver.kt b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetySourceReceiver.kt
index 8e30164..6e14e31 100644
--- a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetySourceReceiver.kt
+++ b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/SafetySourceReceiver.kt
@@ -16,9 +16,7 @@
package android.safetycenter.cts.testing
-import android.Manifest.permission.MANAGE_SAFETY_CENTER
import android.Manifest.permission.SEND_SAFETY_CENTER_UPDATE
-import android.Manifest.permission.WRITE_DEVICE_CONFIG
import android.content.BroadcastReceiver
import android.content.Context
import android.content.Intent
@@ -38,7 +36,9 @@
import android.safetycenter.SafetySourceErrorDetails
import android.safetycenter.cts.testing.Coroutines.TIMEOUT_LONG
import android.safetycenter.cts.testing.Coroutines.runBlockingWithTimeout
-import com.android.compatibility.common.util.SystemUtil.callWithShellPermissionIdentity
+import android.safetycenter.cts.testing.SafetyCenterApisWithShellPermissions.executeSafetyCenterIssueActionWithPermission
+import android.safetycenter.cts.testing.SafetyCenterApisWithShellPermissions.refreshSafetySourcesWithPermission
+import android.safetycenter.cts.testing.ShellPermissions.callWithShellPermissionIdentity
import java.time.Duration
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.channels.Channel.Factory.UNLIMITED
@@ -175,11 +175,10 @@
) =
callWithShellPermissionIdentity(
{
- refreshSafetySources(refreshReason)
+ refreshSafetySourcesWithPermission(refreshReason)
receiveRefreshSafetySources(timeout)
},
- SEND_SAFETY_CENTER_UPDATE,
- MANAGE_SAFETY_CENTER)
+ SEND_SAFETY_CENTER_UPDATE)
fun setSafetyCenterEnabledWithReceiverPermissionAndWait(
value: Boolean,
@@ -187,11 +186,10 @@
) =
callWithShellPermissionIdentity(
{
- SafetyCenterFlags.setSafetyCenterEnabledWithoutPermission(value)
+ SafetyCenterFlags.isEnabled = value
receiveSafetyCenterEnabledChanged(timeout)
},
- SEND_SAFETY_CENTER_UPDATE,
- WRITE_DEVICE_CONFIG)
+ SEND_SAFETY_CENTER_UPDATE)
fun SafetyCenterManager.executeSafetyCenterIssueActionWithReceiverPermissionAndWait(
issueId: String,
@@ -200,11 +198,10 @@
) =
callWithShellPermissionIdentity(
{
- executeSafetyCenterIssueAction(issueId, issueActionId)
+ executeSafetyCenterIssueActionWithPermission(issueId, issueActionId)
receiveInlineAction(timeout)
},
- SEND_SAFETY_CENTER_UPDATE,
- MANAGE_SAFETY_CENTER)
+ SEND_SAFETY_CENTER_UPDATE)
private fun createRefreshEvent(broadcastId: String) =
SafetyEvent.Builder(SAFETY_EVENT_TYPE_REFRESH_REQUESTED)
diff --git a/tests/cts/safetycenter/src/android/safetycenter/cts/testing/ShellPermissions.kt b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/ShellPermissions.kt
new file mode 100644
index 0000000..957e045
--- /dev/null
+++ b/tests/cts/safetycenter/src/android/safetycenter/cts/testing/ShellPermissions.kt
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.safetycenter.cts.testing
+
+import android.app.UiAutomation
+import android.app.UiAutomation.ALL_PERMISSIONS
+import android.safetycenter.cts.testing.ShellPermissions.adoptShellPermissionIdentityFor
+import androidx.annotation.GuardedBy
+import androidx.test.InstrumentationRegistry.getInstrumentation
+import com.android.compatibility.common.util.SystemUtil
+import com.google.common.collect.HashMultiset
+import com.google.common.collect.Multiset
+
+/** A class to facilitate working with System Shell permissions. */
+object ShellPermissions {
+
+ private val lock = Object()
+ @GuardedBy("lock") private val nestedPermissions: Multiset<String> = HashMultiset.create()
+
+ /**
+ * Behaves the same way as [SystemUtil.callWithShellPermissionIdentity], but allows nesting
+ * calls to it by merging the adopted [permissions] within each [block].
+ *
+ * Note that [SystemUtil.callWithShellPermissionIdentity] should NOT be used together with this
+ * method.
+ */
+ fun <T> callWithShellPermissionIdentity(block: () -> T, vararg permissions: String): T {
+ val uiAutomation = getInstrumentation().getUiAutomation()
+ val permissionsToAddForThisBlock =
+ if (permissions.isEmpty()) {
+ ALL_PERMISSIONS
+ } else {
+ permissions.toSet()
+ }
+ synchronized(lock) {
+ permissionsToAddForThisBlock.forEach { nestedPermissions.add(it) }
+ uiAutomation.adoptShellPermissionIdentityFor(nestedPermissions.elementSet())
+ }
+ try {
+ return block()
+ } finally {
+ synchronized(lock) {
+ permissionsToAddForThisBlock.forEach { nestedPermissions.remove(it) }
+ if (nestedPermissions.isEmpty()) {
+ uiAutomation.dropShellPermissionIdentity()
+ } else {
+ uiAutomation.adoptShellPermissionIdentityFor(nestedPermissions.elementSet())
+ }
+ }
+ }
+ }
+
+ private fun UiAutomation.adoptShellPermissionIdentityFor(permissionsToAdopt: Set<String>) {
+ if (permissionsToAdopt.containsAll(ALL_PERMISSIONS)) {
+ adoptShellPermissionIdentity()
+ } else {
+ adoptShellPermissionIdentity(*permissionsToAdopt.toTypedArray())
+ }
+ }
+}
