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

* changes:
  [automerge] Make unregister receiver "sync". 2p: 8745729592
  Make unregister receiver "sync".

packages/SystemUI/src/com/android/systemui/broadcast/ActionReceiver.kt

@@ -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)

packages/SystemUI/src/com/android/systemui/broadcast/BroadcastDispatcher.kt

@@ -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)
             }

packages/SystemUI/src/com/android/systemui/broadcast/PendingRemovalStore.kt

@@ -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))
+                }
+            }
+        }
+    }
+}

packages/SystemUI/src/com/android/systemui/broadcast/UserBroadcastDispatcher.kt

@@ -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 {

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

@@ -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, {

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

@@ -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))

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

@@ -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))
         }

packages/SystemUI/tests/src/com/android/systemui/broadcast/PendingRemovalStoreTest.kt

@@ -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()
+    }
+}

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

@@ -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(