Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge changes from topic "presubmit-am-26a6a382dfac41a4a9c6ec4b78bbc581" into tm-mainline-prod

Browse Source 
* changes:
  [automerge] Make unregister receiver "sync". 2p: 8745729592
  Make unregister receiver "sync".
This commit is contained in:
TreeHugger Robot
2022-04-12 16:23:02 +00:00
committed by Android (Google) Code Review
parent c2a4128e37 afe3e627a2
commit d24b643825
9 changed files with 309 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
packages/SystemUI/src/com/android/systemui/broadcast/ActionReceiver.kt
View File

@@ -39,6 +39,13 @@ import java.util.concurrent.atomic.AtomicInteger
*
* This class has no sync controls, so make sure to only make modifications from the background
* thread.
*
* This class takes the following actions:
* * [registerAction]: action to register this receiver (with the proper filter) with [Context].
* * [unregisterAction]: action to unregister this receiver with [Context].
* * [testPendingRemovalAction]: action to check if a particular [BroadcastReceiver] registered
* with [BroadcastDispatcher] has been unregistered and is pending removal. See
* [PendingRemovalStore].
*/
class ActionReceiver(
private val action: String,
@@ -46,7 +53,8 @@ class ActionReceiver(
private val registerAction: BroadcastReceiver.(IntentFilter) -> Unit,
private val unregisterAction: BroadcastReceiver.() -> Unit,
private val bgExecutor: Executor,
private val logger: BroadcastDispatcherLogger
private val logger: BroadcastDispatcherLogger,
private val testPendingRemovalAction: (BroadcastReceiver, Int) -> Boolean
) : BroadcastReceiver(), Dumpable {
companion object {
@@ -106,7 +114,8 @@ class ActionReceiver(
// Immediately return control to ActivityManager
bgExecutor.execute {
receiverDatas.forEach {
if (it.filter.matchCategories(intent.categories) == null) {
if (it.filter.matchCategories(intent.categories) == null &&
!testPendingRemovalAction(it.receiver, userId)) {
it.executor.execute {
it.receiver.pendingResult = pendingResult
it.receiver.onReceive(context, intent)

30
packages/SystemUI/src/com/android/systemui/broadcast/BroadcastDispatcher.kt
View File

@@ -63,13 +63,14 @@ private const val DEBUG = true
* Broadcast handling may be asynchronous *without* calling goAsync(), as it's running within sysui
* and doesn't need to worry about being killed.
*/
open class BroadcastDispatcher constructor (
open class BroadcastDispatcher @JvmOverloads constructor (
private val context: Context,
private val bgLooper: Looper,
private val bgExecutor: Executor,
private val dumpManager: DumpManager,
private val logger: BroadcastDispatcherLogger,
private val userTracker: UserTracker
private val userTracker: UserTracker,
private val removalPendingStore: PendingRemovalStore = PendingRemovalStore(logger)
) : Dumpable {
// Only modify in BG thread
@@ -167,6 +168,7 @@ open class BroadcastDispatcher constructor (
* @param receiver The receiver to unregister. It will be unregistered for all users.
*/
open fun unregisterReceiver(receiver: BroadcastReceiver) {
removalPendingStore.tagForRemoval(receiver, UserHandle.USER_ALL)
handler.obtainMessage(MSG_REMOVE_RECEIVER, receiver).sendToTarget()
}
@@ -177,13 +179,21 @@ open class BroadcastDispatcher constructor (
* @param user The user associated to the registered [receiver]. It can be [UserHandle.ALL].
*/
open fun unregisterReceiverForUser(receiver: BroadcastReceiver, user: UserHandle) {
removalPendingStore.tagForRemoval(receiver, user.identifier)
handler.obtainMessage(MSG_REMOVE_RECEIVER_FOR_USER, user.identifier, 0, receiver)
.sendToTarget()
}
@VisibleForTesting
protected open fun createUBRForUser(userId: Int) =
UserBroadcastDispatcher(context, userId, bgLooper, bgExecutor, logger)
UserBroadcastDispatcher(
context,
userId,
bgLooper,
bgExecutor,
logger,
removalPendingStore
)
override fun dump(pw: PrintWriter, args: Array<out String>) {
pw.println("Broadcast dispatcher:")
@@ -193,6 +203,8 @@ open class BroadcastDispatcher constructor (
ipw.println("User ${receiversByUser.keyAt(index)}")
receiversByUser.valueAt(index).dump(ipw, args)
}
ipw.println("Pending removal:")
removalPendingStore.dump(ipw, args)
ipw.decreaseIndent()
}
@@ -223,10 +235,20 @@ open class BroadcastDispatcher constructor (
for (it in 0 until receiversByUser.size()) {
receiversByUser.valueAt(it).unregisterReceiver(msg.obj as BroadcastReceiver)
}
removalPendingStore.clearPendingRemoval(
msg.obj as BroadcastReceiver,
UserHandle.USER_ALL
)
}
MSG_REMOVE_RECEIVER_FOR_USER -> {
receiversByUser.get(msg.arg1)?.unregisterReceiver(msg.obj as BroadcastReceiver)
val userId = if (msg.arg1 == UserHandle.USER_CURRENT) {
userTracker.userId
} else {
msg.arg1
}
receiversByUser.get(userId)?.unregisterReceiver(msg.obj as BroadcastReceiver)
removalPendingStore.clearPendingRemoval(msg.obj as BroadcastReceiver, userId)
}
else -> super.handleMessage(msg)
}

58
packages/SystemUI/src/com/android/systemui/broadcast/PendingRemovalStore.kt Normal file
View File

@@ -0,0 +1,58 @@
package com.android.systemui.broadcast
import android.content.BroadcastReceiver
import android.os.UserHandle
import android.util.SparseSetArray
import androidx.annotation.GuardedBy
import com.android.systemui.Dumpable
import com.android.systemui.broadcast.logging.BroadcastDispatcherLogger
import com.android.systemui.util.indentIfPossible
import java.io.PrintWriter
/**
* Store information about requests for unregistering receivers from [BroadcastDispatcher], before
* they have been completely removed from the system.
*
* This helps make unregistering a receiver a *sync* operation.
*/
class PendingRemovalStore(
private val logger: BroadcastDispatcherLogger
) : Dumpable {
@GuardedBy("pendingRemoval")
private val pendingRemoval: SparseSetArray<BroadcastReceiver> = SparseSetArray()
fun tagForRemoval(broadcastReceiver: BroadcastReceiver, userId: Int) {
logger.logTagForRemoval(userId, broadcastReceiver)
synchronized(pendingRemoval) {
pendingRemoval.add(userId, broadcastReceiver)
}
}
fun isPendingRemoval(broadcastReceiver: BroadcastReceiver, userId: Int): Boolean {
return synchronized(pendingRemoval) {
pendingRemoval.contains(userId, broadcastReceiver) ||
pendingRemoval.contains(UserHandle.USER_ALL, broadcastReceiver)
}
}
fun clearPendingRemoval(broadcastReceiver: BroadcastReceiver, userId: Int) {
synchronized(pendingRemoval) {
pendingRemoval.remove(userId, broadcastReceiver)
}
logger.logClearedAfterRemoval(userId, broadcastReceiver)
}
override fun dump(pw: PrintWriter, args: Array<out String>) {
synchronized(pendingRemoval) {
pw.indentIfPossible {
val size = pendingRemoval.size()
for (i in 0 until size) {
val user = pendingRemoval.keyAt(i)
print(user)
print("->")
println(pendingRemoval.get(user))
}
}
}
}
}

32
packages/SystemUI/src/com/android/systemui/broadcast/UserBroadcastDispatcher.kt
View File

@@ -20,12 +20,12 @@ import android.content.BroadcastReceiver
import android.content.Context
import android.os.Handler
import android.os.Looper
import android.os.Message
import android.os.UserHandle
import android.util.ArrayMap
import android.util.ArraySet
import android.util.Log
import androidx.annotation.VisibleForTesting
import androidx.annotation.WorkerThread
import com.android.internal.util.Preconditions
import com.android.systemui.Dumpable
import com.android.systemui.broadcast.logging.BroadcastDispatcherLogger
@@ -34,8 +34,6 @@ import java.io.PrintWriter
import java.util.concurrent.Executor
import java.util.concurrent.atomic.AtomicInteger
private const val MSG_REGISTER_RECEIVER = 0
private const val MSG_UNREGISTER_RECEIVER = 1
private const val TAG = "UserBroadcastDispatcher"
private const val DEBUG = false
@@ -50,7 +48,8 @@ open class UserBroadcastDispatcher(
private val userId: Int,
private val bgLooper: Looper,
private val bgExecutor: Executor,
private val logger: BroadcastDispatcherLogger
private val logger: BroadcastDispatcherLogger,
private val removalPendingStore: PendingRemovalStore
) : Dumpable {
companion object {
@@ -60,16 +59,6 @@ open class UserBroadcastDispatcher(
val index = AtomicInteger(0)
}
private val bgHandler = object : Handler(bgLooper) {
override fun handleMessage(msg: Message) {
when (msg.what) {
MSG_REGISTER_RECEIVER -> handleRegisterReceiver(msg.obj as ReceiverData, msg.arg1)
MSG_UNREGISTER_RECEIVER -> handleUnregisterReceiver(msg.obj as BroadcastReceiver)
else -> Unit
}
}
}
// Used for key in actionsToActionsReceivers
internal data class ReceiverProperties(
val action: String,
@@ -77,6 +66,8 @@ open class UserBroadcastDispatcher(
val permission: String?
)
private val bgHandler = Handler(bgLooper)
// Only modify in BG thread
@VisibleForTesting
internal val actionsToActionsReceivers = ArrayMap<ReceiverProperties, ActionReceiver>()
@@ -92,19 +83,21 @@ open class UserBroadcastDispatcher(
/**
* Register a [ReceiverData] for this user.
*/
@WorkerThread
fun registerReceiver(receiverData: ReceiverData, flags: Int) {
bgHandler.obtainMessage(MSG_REGISTER_RECEIVER, flags, 0, receiverData).sendToTarget()
handleRegisterReceiver(receiverData, flags)
}
/**
* Unregister a given [BroadcastReceiver] for this user.
*/
@WorkerThread
fun unregisterReceiver(receiver: BroadcastReceiver) {
bgHandler.obtainMessage(MSG_UNREGISTER_RECEIVER, receiver).sendToTarget()
handleUnregisterReceiver(receiver)
}
private fun handleRegisterReceiver(receiverData: ReceiverData, flags: Int) {
Preconditions.checkState(bgHandler.looper.isCurrentThread,
Preconditions.checkState(bgLooper.isCurrentThread,
"This method should only be called from BG thread")
if (DEBUG) Log.w(TAG, "Register receiver: ${receiverData.receiver}")
receiverToActions
@@ -151,12 +144,13 @@ open class UserBroadcastDispatcher(
}
},
bgExecutor,
logger
logger,
removalPendingStore::isPendingRemoval
)
}
private fun handleUnregisterReceiver(receiver: BroadcastReceiver) {
Preconditions.checkState(bgHandler.looper.isCurrentThread,
Preconditions.checkState(bgLooper.isCurrentThread,
"This method should only be called from BG thread")
if (DEBUG) Log.w(TAG, "Unregister receiver: $receiver")
receiverToActions.getOrDefault(receiver, mutableSetOf()).forEach {

20
packages/SystemUI/src/com/android/systemui/broadcast/logging/BroadcastDispatcherLogger.kt
View File

@@ -87,6 +87,26 @@ class BroadcastDispatcherLogger @Inject constructor(
})
}
fun logTagForRemoval(user: Int, receiver: BroadcastReceiver) {
val receiverString = receiver.toString()
log(DEBUG, {
int1 = user
str1 = receiverString
}, {
"Receiver $str1 tagged for removal from user $int1"
})
}
fun logClearedAfterRemoval(user: Int, receiver: BroadcastReceiver) {
val receiverString = receiver.toString()
log(DEBUG, {
int1 = user
str1 = receiverString
}, {
"Receiver $str1 has been completely removed for user $int1"
})
}
fun logReceiverUnregistered(user: Int, receiver: BroadcastReceiver) {
val receiverString = receiver.toString()
log(INFO, {

22
packages/SystemUI/tests/src/com/android/systemui/broadcast/ActionReceiverTest.kt
View File

@@ -42,6 +42,7 @@ import org.mockito.ArgumentMatchers.anyString
import org.mockito.Captor
import org.mockito.Mock
import org.mockito.Mockito
import org.mockito.Mockito.`when`
import org.mockito.Mockito.mock
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
@@ -81,6 +82,8 @@ class ActionReceiverTest : SysuiTestCase() {
@Mock
private lateinit var unregisterFunction: BroadcastReceiver.() -> Unit
@Mock
private lateinit var isPendingRemovalFunction: (BroadcastReceiver, Int) -> Boolean
@Mock
private lateinit var receiver1: BroadcastReceiver
@Mock
private lateinit var receiver2: BroadcastReceiver
@@ -98,13 +101,16 @@ class ActionReceiverTest : SysuiTestCase() {
MockitoAnnotations.initMocks(this)
executor = FakeExecutor(FakeSystemClock())
`when`(isPendingRemovalFunction(any(), anyInt())).thenReturn(false)
actionReceiver = ActionReceiver(
ACTION1,
USER.identifier,
registerFunction,
unregisterFunction,
executor,
logger
logger,
isPendingRemovalFunction
)
}
@@ -249,6 +255,20 @@ class ActionReceiverTest : SysuiTestCase() {
verify(logger).logBroadcastDispatched(anyInt(), eq(ACTION1), sameNotNull(receiver1))
}
@Test
fun testBroadcastNotDispatchingOnPendingRemoval() {
`when`(isPendingRemovalFunction(receiver1, USER.identifier)).thenReturn(true)
val receiverData = ReceiverData(receiver1, IntentFilter(ACTION1), directExecutor, USER)
actionReceiver.addReceiverData(receiverData)
val intent = Intent(ACTION1)
actionReceiver.onReceive(mContext, intent)
executor.runAllReady()
verify(receiver1, never()).onReceive(any(), eq(intent))
}
@Test(expected = IllegalStateException::class)
fun testBroadcastWithWrongAction_throwsException() {
actionReceiver.onReceive(mContext, Intent(ACTION2))

67
packages/SystemUI/tests/src/com/android/systemui/broadcast/BroadcastDispatcherTest.kt
View File

@@ -41,6 +41,8 @@ import org.mockito.ArgumentCaptor
import org.mockito.Captor
import org.mockito.Mock
import org.mockito.Mockito.`when`
import org.mockito.Mockito.anyInt
import org.mockito.Mockito.inOrder
import org.mockito.Mockito.mock
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
@@ -85,6 +87,8 @@ class BroadcastDispatcherTest : SysuiTestCase() {
private lateinit var logger: BroadcastDispatcherLogger
@Mock
private lateinit var userTracker: UserTracker
@Mock
private lateinit var removalPendingStore: PendingRemovalStore
private lateinit var executor: Executor
@@ -108,6 +112,7 @@ class BroadcastDispatcherTest : SysuiTestCase() {
mock(DumpManager::class.java),
logger,
userTracker,
removalPendingStore,
mapOf(0 to mockUBRUser0, 1 to mockUBRUser1))
// These should be valid filters
@@ -325,6 +330,57 @@ class BroadcastDispatcherTest : SysuiTestCase() {
broadcastDispatcher.registerReceiver(broadcastReceiver, testFilter)
}
@Test
fun testTaggedReceiverForRemovalImmediately_allUsers() {
broadcastDispatcher.unregisterReceiver(broadcastReceiver)
verify(removalPendingStore).tagForRemoval(broadcastReceiver, UserHandle.USER_ALL)
verify(removalPendingStore, never()).clearPendingRemoval(eq(broadcastReceiver), anyInt())
}
@Test
fun testTaggedReceiverForRemovalImmediately_singleUser() {
val user = 0
broadcastDispatcher.unregisterReceiverForUser(broadcastReceiver, UserHandle.of(user))
verify(removalPendingStore).tagForRemoval(broadcastReceiver, user)
verify(removalPendingStore, never()).clearPendingRemoval(eq(broadcastReceiver), anyInt())
}
@Test
fun testUnregisterReceiverClearsPendingRemovalAfterRemoving_allUsers() {
broadcastDispatcher.registerReceiver(broadcastReceiver, intentFilter, null, user0)
broadcastDispatcher.registerReceiver(broadcastReceiver, intentFilter, null, user1)
broadcastDispatcher.unregisterReceiver(broadcastReceiver)
testableLooper.processAllMessages()
val inOrderUser0 = inOrder(mockUBRUser0, removalPendingStore)
inOrderUser0.verify(mockUBRUser0).unregisterReceiver(broadcastReceiver)
inOrderUser0.verify(removalPendingStore)
.clearPendingRemoval(broadcastReceiver, UserHandle.USER_ALL)
val inOrderUser1 = inOrder(mockUBRUser1, removalPendingStore)
inOrderUser1.verify(mockUBRUser1).unregisterReceiver(broadcastReceiver)
inOrderUser1.verify(removalPendingStore)
.clearPendingRemoval(broadcastReceiver, UserHandle.USER_ALL)
}
@Test
fun testUnregisterReceiverclearPendingRemovalAfterRemoving_singleUser() {
broadcastDispatcher.registerReceiver(broadcastReceiver, intentFilter, null, user1)
broadcastDispatcher.unregisterReceiverForUser(broadcastReceiver, user1)
testableLooper.processAllMessages()
val inOrderUser1 = inOrder(mockUBRUser1, removalPendingStore)
inOrderUser1.verify(mockUBRUser1).unregisterReceiver(broadcastReceiver)
inOrderUser1.verify(removalPendingStore)
.clearPendingRemoval(broadcastReceiver, user1.identifier)
}
private fun setUserMock(mockContext: Context, user: UserHandle) {
`when`(mockContext.user).thenReturn(user)
`when`(mockContext.userId).thenReturn(user.identifier)
@@ -337,8 +393,17 @@ class BroadcastDispatcherTest : SysuiTestCase() {
dumpManager: DumpManager,
logger: BroadcastDispatcherLogger,
userTracker: UserTracker,
removalPendingStore: PendingRemovalStore,
var mockUBRMap: Map<Int, UserBroadcastDispatcher>
) : BroadcastDispatcher(context, bgLooper, executor, dumpManager, logger, userTracker) {
) : BroadcastDispatcher(
context,
bgLooper,
executor,
dumpManager,
logger,
userTracker,
removalPendingStore
) {
override fun createUBRForUser(userId: Int): UserBroadcastDispatcher {
return mockUBRMap.getOrDefault(userId, mock(UserBroadcastDispatcher::class.java))
}

81
packages/SystemUI/tests/src/com/android/systemui/broadcast/PendingRemovalStoreTest.kt Normal file
View File

@@ -0,0 +1,81 @@
package com.android.systemui.broadcast
import android.content.BroadcastReceiver
import android.os.UserHandle
import android.testing.AndroidTestingRunner
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.broadcast.logging.BroadcastDispatcherLogger
import com.google.common.truth.Truth.assertThat
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.Mockito.verify
import org.mockito.MockitoAnnotations
@RunWith(AndroidTestingRunner::class)
@SmallTest
class PendingRemovalStoreTest : SysuiTestCase() {
@Mock
private lateinit var logger: BroadcastDispatcherLogger
@Mock
private lateinit var receiverOne: BroadcastReceiver
@Mock
private lateinit var receiverTwo: BroadcastReceiver
private lateinit var store: PendingRemovalStore
@Before
fun setUp() {
MockitoAnnotations.initMocks(this)
store = PendingRemovalStore(logger)
}
@Test
fun testTagForRemoval_logged() {
val user = 10
store.tagForRemoval(receiverOne, 10)
verify(logger).logTagForRemoval(user, receiverOne)
}
@Test
fun testClearedPendingRemoval_logged() {
val user = UserHandle.USER_ALL
store.clearPendingRemoval(receiverOne, user)
verify(logger).logClearedAfterRemoval(user, receiverOne)
}
@Test
fun testTaggedReceiverMarkedAsPending_specificUser() {
val user = 10
store.tagForRemoval(receiverOne, user)
assertThat(store.isPendingRemoval(receiverOne, user)).isTrue()
assertThat(store.isPendingRemoval(receiverOne, user + 1)).isFalse()
assertThat(store.isPendingRemoval(receiverOne, UserHandle.USER_ALL)).isFalse()
}
@Test
fun testTaggedReceiverMarkedAsPending_allUsers() {
val user = 10
store.tagForRemoval(receiverOne, UserHandle.USER_ALL)
assertThat(store.isPendingRemoval(receiverOne, user)).isTrue()
assertThat(store.isPendingRemoval(receiverOne, user + 1)).isTrue()
assertThat(store.isPendingRemoval(receiverOne, UserHandle.USER_ALL)).isTrue()
}
@Test
fun testOnlyBlockCorrectReceiver() {
val user = 10
store.tagForRemoval(receiverOne, user)
assertThat(store.isPendingRemoval(receiverOne, user)).isTrue()
assertThat(store.isPendingRemoval(receiverTwo, user)).isFalse()
}
}

16
packages/SystemUI/tests/src/com/android/systemui/broadcast/UserBroadcastDispatcherTest.kt
View File

@@ -68,6 +68,8 @@ class UserBroadcastDispatcherTest : SysuiTestCase() {
private lateinit var mockContext: Context
@Mock
private lateinit var logger: BroadcastDispatcherLogger
@Mock
private lateinit var removalPendingStore: PendingRemovalStore
private lateinit var testableLooper: TestableLooper
private lateinit var userBroadcastDispatcher: UserBroadcastDispatcher
@@ -84,7 +86,13 @@ class UserBroadcastDispatcherTest : SysuiTestCase() {
fakeExecutor = FakeExecutor(FakeSystemClock())
userBroadcastDispatcher = object : UserBroadcastDispatcher(
mockContext, USER_ID, testableLooper.looper, mock(Executor::class.java), logger) {
mockContext,
USER_ID,
testableLooper.looper,
mock(Executor::class.java),
logger,
removalPendingStore
) {
override fun createActionReceiver(
action: String,
permission: String?,
@@ -216,7 +224,8 @@ class UserBroadcastDispatcherTest : SysuiTestCase() {
USER_ID,
testableLooper.looper,
fakeExecutor,
logger
logger,
removalPendingStore
)
uBR.registerReceiver(
ReceiverData(
@@ -243,7 +252,8 @@ class UserBroadcastDispatcherTest : SysuiTestCase() {
USER_ID,
testableLooper.looper,
fakeExecutor,
logger
logger,
removalPendingStore
)
uBR.registerReceiver(
ReceiverData(