Create new sysui thread for using broadcasts

Also, - Replace getMainExectutor() with injected @Main Executor - Fix incorrect "by default" comment - Remove unused DEBUG val Test: manual, inspect perfetto trace Bug: 238923086 Bug: 251771411 Change-Id: Idd6099c67c51dc0814ca371cad1fc9207c4ccfbb
2022-10-06 14:55:15 -07:00
parent 3e5c74adf4
commit dcea61ac55
8 changed files with 105 additions and 40 deletions
--- a/packages/SystemUI/src/com/android/systemui/broadcast/ActionReceiver.kt
+++ b/packages/SystemUI/src/com/android/systemui/broadcast/ActionReceiver.kt
@@ -52,7 +52,7 @@ class ActionReceiver(
    private val userId: Int,
    private val registerAction: BroadcastReceiver.(IntentFilter) -> Unit,
    private val unregisterAction: BroadcastReceiver.() -> Unit,
-    private val bgExecutor: Executor,
+    private val workerExecutor: Executor,
    private val logger: BroadcastDispatcherLogger,
    private val testPendingRemovalAction: (BroadcastReceiver, Int) -> Boolean
 ) : BroadcastReceiver(), Dumpable {
@@ -112,7 +112,7 @@ class ActionReceiver(
        val id = index.getAndIncrement()
        logger.logBroadcastReceived(id, userId, intent)
        // Immediately return control to ActivityManager
-        bgExecutor.execute {
+        workerExecutor.execute {
            receiverDatas.forEach {
                if (it.filter.matchCategories(intent.categories) == null &&
                    !testPendingRemovalAction(it.receiver, userId)) {
@@ -138,4 +138,4 @@ class ActionReceiver(
            println("Categories: ${activeCategories.joinToString(", ")}")
        }
    }
-}
+}
--- a/packages/SystemUI/src/com/android/systemui/broadcast/BroadcastDispatcher.kt
+++ b/packages/SystemUI/src/com/android/systemui/broadcast/BroadcastDispatcher.kt
@@ -34,7 +34,8 @@ import com.android.systemui.broadcast.logging.BroadcastDispatcherLogger
 import com.android.systemui.common.coroutine.ChannelExt.trySendWithFailureLogging
 import com.android.systemui.common.coroutine.ConflatedCallbackFlow.conflatedCallbackFlow
 import com.android.systemui.dagger.SysUISingleton
-import com.android.systemui.dagger.qualifiers.Background
+import com.android.systemui.dagger.qualifiers.BroadcastRunning
+import com.android.systemui.dagger.qualifiers.Main
 import com.android.systemui.dump.DumpManager
 import com.android.systemui.settings.UserTracker
 import java.io.PrintWriter
@@ -55,7 +56,6 @@ private const val MSG_ADD_RECEIVER = 0
 private const val MSG_REMOVE_RECEIVER = 1
 private const val MSG_REMOVE_RECEIVER_FOR_USER = 2
 private const val TAG = "BroadcastDispatcher"
-private const val DEBUG = true

 /**
 * SystemUI master Broadcast Dispatcher.
@@ -73,15 +73,16 @@ private const val DEBUG = true
@SysUISingleton
 open class BroadcastDispatcher @Inject constructor(
    private val context: Context,
-    @Background private val bgLooper: Looper,
-    @Background private val bgExecutor: Executor,
+    @Main private val mainExecutor: Executor,
+    @BroadcastRunning private val broadcastLooper: Looper,
+    @BroadcastRunning private val broadcastExecutor: Executor,
    private val dumpManager: DumpManager,
    private val logger: BroadcastDispatcherLogger,
    private val userTracker: UserTracker,
    private val removalPendingStore: PendingRemovalStore
 ) : Dumpable {

-    // Only modify in BG thread
+    // Only modify in BroadcastRunning thread
    private val receiversByUser = SparseArray<UserBroadcastDispatcher>(20)

    fun initialize() {
@@ -148,7 +149,7 @@ open class BroadcastDispatcher @Inject constructor(
        val data = ReceiverData(
                receiver,
                filter,
-                executor ?: context.mainExecutor,
+                executor ?: mainExecutor,
                user ?: context.user,
                permission
            )
@@ -181,7 +182,7 @@ open class BroadcastDispatcher @Inject constructor(
        registerReceiver(
            receiver,
            filter,
-            bgExecutor,
+            broadcastExecutor,
            user,
            flags,
            permission,
@@ -246,8 +247,8 @@ open class BroadcastDispatcher @Inject constructor(
            UserBroadcastDispatcher(
                context,
                userId,
-                bgLooper,
-                bgExecutor,
+                broadcastLooper,
+                broadcastExecutor,
                logger,
                removalPendingStore
            )
@@ -265,7 +266,7 @@ open class BroadcastDispatcher @Inject constructor(
        ipw.decreaseIndent()
    }

-    private val handler = object : Handler(bgLooper) {
+    private val handler = object : Handler(broadcastLooper) {

        override fun handleMessage(msg: Message) {
            when (msg.what) {
--- a/packages/SystemUI/src/com/android/systemui/broadcast/UserBroadcastDispatcher.kt
+++ b/packages/SystemUI/src/com/android/systemui/broadcast/UserBroadcastDispatcher.kt
@@ -16,6 +16,7 @@

 package com.android.systemui.broadcast

+import android.annotation.SuppressLint
 import android.content.BroadcastReceiver
 import android.content.Context
 import android.os.Handler
@@ -46,8 +47,8 @@ private const val DEBUG = false
 open class UserBroadcastDispatcher(
    private val context: Context,
    private val userId: Int,
-    private val bgLooper: Looper,
-    private val bgExecutor: Executor,
+    private val workerLooper: Looper,
+    private val workerExecutor: Executor,
    private val logger: BroadcastDispatcherLogger,
    private val removalPendingStore: PendingRemovalStore
 ) : Dumpable {
@@ -66,9 +67,11 @@ open class UserBroadcastDispatcher(
        val permission: String?
    )

-    private val bgHandler = Handler(bgLooper)
+    private val wrongThreadErrorMsg = "This method should only be called from the worker thread " +
+            "(which is expected to be the BroadcastRunning thread)"
+    private val workerHandler = Handler(workerLooper)

-    // Only modify in BG thread
+    // Only modify in BroadcastRunning thread
    @VisibleForTesting
    internal val actionsToActionsReceivers = ArrayMap<ReceiverProperties, ActionReceiver>()
    private val receiverToActions = ArrayMap<BroadcastReceiver, MutableSet<String>>()
@@ -97,8 +100,7 @@ open class UserBroadcastDispatcher(
    }

    private fun handleRegisterReceiver(receiverData: ReceiverData, flags: Int) {
-        Preconditions.checkState(bgLooper.isCurrentThread,
-                "This method should only be called from BG thread")
+        Preconditions.checkState(workerLooper.isCurrentThread, wrongThreadErrorMsg)
        if (DEBUG) Log.w(TAG, "Register receiver: ${receiverData.receiver}")
        receiverToActions
                .getOrPut(receiverData.receiver, { ArraySet() })
@@ -113,6 +115,7 @@ open class UserBroadcastDispatcher(
        logger.logReceiverRegistered(userId, receiverData.receiver, flags)
    }

+    @SuppressLint("RegisterReceiverViaContextDetector")
    @VisibleForTesting
    internal open fun createActionReceiver(
        action: String,
@@ -128,7 +131,7 @@ open class UserBroadcastDispatcher(
                            UserHandle.of(userId),
                            it,
                            permission,
-                            bgHandler,
+                            workerHandler,
                            flags
                    )
                    logger.logContextReceiverRegistered(userId, flags, it)
@@ -143,15 +146,14 @@ open class UserBroadcastDispatcher(
                                IllegalStateException(e))
                    }
                },
-                bgExecutor,
+                workerExecutor,
                logger,
                removalPendingStore::isPendingRemoval
        )
    }

    private fun handleUnregisterReceiver(receiver: BroadcastReceiver) {
-        Preconditions.checkState(bgLooper.isCurrentThread,
-                "This method should only be called from BG thread")
+        Preconditions.checkState(workerLooper.isCurrentThread, wrongThreadErrorMsg)
        if (DEBUG) Log.w(TAG, "Unregister receiver: $receiver")
        receiverToActions.getOrDefault(receiver, mutableSetOf()).forEach {
            actionsToActionsReceivers.forEach { (key, value) ->
--- a/packages/SystemUI/src/com/android/systemui/dagger/qualifiers/BroadcastRunning.java
+++ b/packages/SystemUI/src/com/android/systemui/dagger/qualifiers/BroadcastRunning.java
@@ -0,0 +1,30 @@
+/*
+ * 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 com.android.systemui.dagger.qualifiers;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+
+import javax.inject.Qualifier;
+
+@Qualifier
+@Documented
+@Retention(RUNTIME)
+public @interface BroadcastRunning {
+}
--- a/packages/SystemUI/src/com/android/systemui/util/concurrency/SysUIConcurrencyModule.java
+++ b/packages/SystemUI/src/com/android/systemui/util/concurrency/SysUIConcurrencyModule.java
@@ -25,6 +25,7 @@ import android.os.Process;

 import com.android.systemui.dagger.SysUISingleton;
 import com.android.systemui.dagger.qualifiers.Background;
+import com.android.systemui.dagger.qualifiers.BroadcastRunning;
 import com.android.systemui.dagger.qualifiers.LongRunning;
 import com.android.systemui.dagger.qualifiers.Main;

@@ -51,6 +52,17 @@ public abstract class SysUIConcurrencyModule {
        return thread.getLooper();
    }

+    /** BroadcastRunning Looper (for sending and receiving broadcasts) */
+    @Provides
+    @SysUISingleton
+    @BroadcastRunning
+    public static Looper provideBroadcastRunningLooper() {
+        HandlerThread thread = new HandlerThread("BroadcastRunning",
+                Process.THREAD_PRIORITY_BACKGROUND);
+        thread.start();
+        return thread.getLooper();
+    }
+
    /** Long running tasks Looper */
    @Provides
    @SysUISingleton
@@ -83,7 +95,17 @@ public abstract class SysUIConcurrencyModule {
    }

    /**
-     * Provide a Long running Executor by default.
+     * Provide a BroadcastRunning Executor (for sending and receiving broadcasts).
+     */
+    @Provides
+    @SysUISingleton
+    @BroadcastRunning
+    public static Executor provideBroadcastRunningExecutor(@BroadcastRunning Looper looper) {
+        return new ExecutorImpl(looper);
+    }
+
+    /**
+     * Provide a Long running Executor.
     */
    @Provides
    @SysUISingleton
--- a/packages/SystemUI/tests/src/com/android/systemui/broadcast/BroadcastDispatcherTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/broadcast/BroadcastDispatcherTest.kt
@@ -96,7 +96,7 @@ class BroadcastDispatcherTest : SysuiTestCase() {
    @Mock
    private lateinit var removalPendingStore: PendingRemovalStore

-    private lateinit var executor: Executor
+    private lateinit var mainExecutor: Executor

    @Captor
    private lateinit var argumentCaptor: ArgumentCaptor<ReceiverData>
@@ -108,11 +108,12 @@ class BroadcastDispatcherTest : SysuiTestCase() {
    fun setUp() {
        MockitoAnnotations.initMocks(this)
        testableLooper = TestableLooper.get(this)
-        executor = FakeExecutor(FakeSystemClock())
-        `when`(mockContext.mainExecutor).thenReturn(executor)
+        mainExecutor = FakeExecutor(FakeSystemClock())
+        `when`(mockContext.mainExecutor).thenReturn(mainExecutor)

        broadcastDispatcher = TestBroadcastDispatcher(
                mockContext,
+                mainExecutor,
                testableLooper.looper,
                mock(Executor::class.java),
                mock(DumpManager::class.java),
@@ -148,9 +149,9 @@ class BroadcastDispatcherTest : SysuiTestCase() {

    @Test
    fun testAddingReceiverToCorrectUBR_executor() {
-        broadcastDispatcher.registerReceiver(broadcastReceiver, intentFilter, executor, user0)
+        broadcastDispatcher.registerReceiver(broadcastReceiver, intentFilter, mainExecutor, user0)
        broadcastDispatcher.registerReceiver(
-                broadcastReceiverOther, intentFilterOther, executor, user1)
+                broadcastReceiverOther, intentFilterOther, mainExecutor, user1)

        testableLooper.processAllMessages()

@@ -427,8 +428,9 @@ class BroadcastDispatcherTest : SysuiTestCase() {

    private class TestBroadcastDispatcher(
        context: Context,
-        bgLooper: Looper,
-        executor: Executor,
+        mainExecutor: Executor,
+        backgroundRunningLooper: Looper,
+        backgroundRunningExecutor: Executor,
        dumpManager: DumpManager,
        logger: BroadcastDispatcherLogger,
        userTracker: UserTracker,
@@ -436,8 +438,9 @@ class BroadcastDispatcherTest : SysuiTestCase() {
        var mockUBRMap: Map<Int, UserBroadcastDispatcher>
    ) : BroadcastDispatcher(
        context,
-        bgLooper,
-        executor,
+        mainExecutor,
+        backgroundRunningLooper,
+        backgroundRunningExecutor,
        dumpManager,
        logger,
        userTracker,
--- a/packages/SystemUI/tests/utils/src/com/android/systemui/SysuiTestCase.java
+++ b/packages/SystemUI/tests/utils/src/com/android/systemui/SysuiTestCase.java
@@ -84,9 +84,14 @@ public abstract class SysuiTestCase {
        initializer.init(true);
        mDependency = new TestableDependency(initializer.getSysUIComponent().createDependency());
        Dependency.setInstance(mDependency);
-        mFakeBroadcastDispatcher = new FakeBroadcastDispatcher(mContext, mock(Looper.class),
-                mock(Executor.class), mock(DumpManager.class),
-                mock(BroadcastDispatcherLogger.class), mock(UserTracker.class));
+        mFakeBroadcastDispatcher = new FakeBroadcastDispatcher(
+                mContext,
+                mContext.getMainExecutor(),
+                mock(Looper.class),
+                mock(Executor.class),
+                mock(DumpManager.class),
+                mock(BroadcastDispatcherLogger.class),
+                mock(UserTracker.class));

        mRealInstrumentation = InstrumentationRegistry.getInstrumentation();
        Instrumentation inst = spy(mRealInstrumentation);
--- a/packages/SystemUI/tests/utils/src/com/android/systemui/broadcast/FakeBroadcastDispatcher.kt
+++ b/packages/SystemUI/tests/utils/src/com/android/systemui/broadcast/FakeBroadcastDispatcher.kt
@@ -32,16 +32,18 @@ import java.util.concurrent.Executor

 class FakeBroadcastDispatcher(
    context: SysuiTestableContext,
-    looper: Looper,
-    executor: Executor,
+    mainExecutor: Executor,
+    broadcastRunningLooper: Looper,
+    broadcastRunningExecutor: Executor,
    dumpManager: DumpManager,
    logger: BroadcastDispatcherLogger,
    userTracker: UserTracker
 ) :
    BroadcastDispatcher(
        context,
-        looper,
-        executor,
+        mainExecutor,
+        broadcastRunningLooper,
+        broadcastRunningExecutor,
        dumpManager,
        logger,
        userTracker,