From 6ca2a219d9d54039e4ffc046778d0314a3f2521b Mon Sep 17 00:00:00 2001 From: Jernej Virag Date: Mon, 7 Mar 2022 12:22:54 +0100 Subject: [PATCH] Remove main thread sendBroadcast calls sendBroadcast ends up with a synchronous Binder call which can block main thread. This moves multiple broadcast calls to background thread to avoid jank. Bug: 234306007 Bug: 221339831 Test: Unit tests Change-Id: Ibfb7ea6302a0723c5c01cae3afe4e66395cd1f50 Merged-In: Ibe76a70e08c9bfefc7ff23806089957f17514c31 --- .../systemui/broadcast/BroadcastSender.kt | 132 ++++++++++++++++ .../systemui/broadcast/BroadcastSenderTest.kt | 145 ++++++++++++++++++ 2 files changed, 277 insertions(+) create mode 100644 packages/SystemUI/src/com/android/systemui/broadcast/BroadcastSender.kt create mode 100644 packages/SystemUI/tests/src/com/android/systemui/broadcast/BroadcastSenderTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/broadcast/BroadcastSender.kt b/packages/SystemUI/src/com/android/systemui/broadcast/BroadcastSender.kt new file mode 100644 index 0000000000000..6615f6b0b9eb8 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/broadcast/BroadcastSender.kt @@ -0,0 +1,132 @@ +package com.android.systemui.broadcast + +import android.annotation.AnyThread +import android.content.Context +import android.content.Intent +import android.os.Bundle +import android.os.UserHandle +import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dagger.qualifiers.Background +import com.android.systemui.util.wakelock.WakeLock +import java.util.concurrent.Executor +import javax.inject.Inject + +/** + * SystemUI master Broadcast sender + * + * This class dispatches broadcasts on background thread to avoid synchronous call to binder. Use + * this class instead of calling [Context.sendBroadcast] directly. + */ +@SysUISingleton +class BroadcastSender @Inject constructor( + private val context: Context, + private val wakeLockBuilder: WakeLock.Builder, + @Background private val bgExecutor: Executor +) { + + private val WAKE_LOCK_TAG = "SysUI:BroadcastSender" + private val WAKE_LOCK_SEND_REASON = "sendInBackground" + + /** + * Sends broadcast via [Context.sendBroadcast] on background thread to avoid blocking + * synchronous binder call. + */ + @AnyThread + fun sendBroadcast(intent: Intent) { + sendInBackground { + context.sendBroadcast(intent) + } + } + + /** + * Sends broadcast via [Context.sendBroadcast] on background thread to avoid blocking + * synchronous binder call. + */ + @AnyThread + fun sendBroadcast(intent: Intent, receiverPermission: String?) { + sendInBackground { + context.sendBroadcast(intent, receiverPermission) + } + } + + /** + * Sends broadcast via [Context.sendBroadcastAsUser] on background thread to avoid blocking + * synchronous binder call. + */ + @AnyThread + fun sendBroadcastAsUser(intent: Intent, userHandle: UserHandle) { + sendInBackground { + context.sendBroadcastAsUser(intent, userHandle) + } + } + + /** + * Sends broadcast via [Context.sendBroadcastAsUser] on background thread to avoid blocking + * synchronous binder call. + */ + @AnyThread + fun sendBroadcastAsUser(intent: Intent, userHandle: UserHandle, receiverPermission: String?) { + sendInBackground { + context.sendBroadcastAsUser(intent, userHandle, receiverPermission) + } + } + + /** + * Sends broadcast via [Context.sendBroadcastAsUser] on background thread to avoid blocking + * synchronous binder call. + */ + @AnyThread + fun sendBroadcastAsUser( + intent: Intent, + userHandle: UserHandle, + receiverPermission: String?, + options: Bundle? + ) { + sendInBackground { + context.sendBroadcastAsUser(intent, userHandle, receiverPermission, options) + } + } + + /** + * Sends broadcast via [Context.sendBroadcastAsUser] on background thread to avoid blocking + * synchronous binder call. + */ + @AnyThread + fun sendBroadcastAsUser( + intent: Intent, + userHandle: UserHandle, + receiverPermission: String?, + appOp: Int + ) { + sendInBackground { + context.sendBroadcastAsUser(intent, userHandle, receiverPermission, appOp) + } + } + + /** + * Sends [Intent.ACTION_CLOSE_SYSTEM_DIALOGS] broadcast to the system. + */ + @AnyThread + fun closeSystemDialogs() { + sendInBackground { + context.sendBroadcast(Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS)) + } + } + + /** + * Dispatches parameter on background executor while holding a wakelock. + */ + private fun sendInBackground(callable: () -> Unit) { + val broadcastWakelock = wakeLockBuilder.setTag(WAKE_LOCK_TAG) + .setMaxTimeout(5000) + .build() + broadcastWakelock.acquire(WAKE_LOCK_SEND_REASON) + bgExecutor.execute { + try { + callable.invoke() + } finally { + broadcastWakelock.release(WAKE_LOCK_SEND_REASON) + } + } + } +} \ No newline at end of file diff --git a/packages/SystemUI/tests/src/com/android/systemui/broadcast/BroadcastSenderTest.kt b/packages/SystemUI/tests/src/com/android/systemui/broadcast/BroadcastSenderTest.kt new file mode 100644 index 0000000000000..fbd2c918648a3 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/broadcast/BroadcastSenderTest.kt @@ -0,0 +1,145 @@ +/* + * 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.broadcast + +import android.content.Context +import android.content.Intent +import android.os.Bundle +import android.os.UserHandle +import android.testing.AndroidTestingRunner +import androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.android.systemui.util.concurrency.FakeExecutor +import com.android.systemui.util.time.FakeSystemClock +import com.android.systemui.util.wakelock.WakeLockFake +import com.google.common.truth.Truth.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor +import org.mockito.Mock +import org.mockito.Mockito.verify +import org.mockito.MockitoAnnotations + +@RunWith(AndroidTestingRunner::class) +@SmallTest +class BroadcastSenderTest : SysuiTestCase() { + + @Mock + private lateinit var mockContext: Context + + private lateinit var broadcastSender: BroadcastSender + private lateinit var executor: FakeExecutor + private lateinit var wakeLock: WakeLockFake + + @Before + fun setUp() { + MockitoAnnotations.initMocks(this) + executor = FakeExecutor(FakeSystemClock()) + wakeLock = WakeLockFake() + val wakeLockBuilder = WakeLockFake.Builder(mContext) + wakeLockBuilder.setWakeLock(wakeLock) + broadcastSender = BroadcastSender(mockContext, wakeLockBuilder, executor) + } + + @Test + fun sendBroadcast_dispatchesWithWakelock() { + val intent = Intent(Intent.ACTION_VIEW) + broadcastSender.sendBroadcast(intent) + + runExecutorAssertingWakelock { + verify(mockContext).sendBroadcast(intent) + } + } + + @Test + fun sendBroadcastWithPermission_dispatchesWithWakelock() { + val intent = Intent(Intent.ACTION_VIEW) + val permission = "Permission" + broadcastSender.sendBroadcast(intent, permission) + + runExecutorAssertingWakelock { + verify(mockContext).sendBroadcast(intent, permission) + } + } + + @Test + fun sendBroadcastAsUser_dispatchesWithWakelock() { + val intent = Intent(Intent.ACTION_VIEW) + broadcastSender.sendBroadcastAsUser(intent, UserHandle.ALL) + + runExecutorAssertingWakelock { + verify(mockContext).sendBroadcastAsUser(intent, UserHandle.ALL) + } + } + + @Test + fun sendBroadcastAsUserWithPermission_dispatchesWithWakelock() { + val intent = Intent(Intent.ACTION_VIEW) + val permission = "Permission" + broadcastSender.sendBroadcastAsUser(intent, UserHandle.ALL, permission) + + runExecutorAssertingWakelock { + verify(mockContext).sendBroadcastAsUser(intent, UserHandle.ALL, permission) + } + } + + @Test + fun sendBroadcastAsUserWithPermissionAndOptions_dispatchesWithWakelock() { + val intent = Intent(Intent.ACTION_VIEW) + val permission = "Permission" + val options = Bundle() + options.putString("key", "value") + + broadcastSender.sendBroadcastAsUser(intent, UserHandle.ALL, permission, options) + + runExecutorAssertingWakelock { + verify(mockContext).sendBroadcastAsUser(intent, UserHandle.ALL, permission, options) + } + } + + @Test + fun sendBroadcastAsUserWithPermissionAndAppOp_dispatchesWithWakelock() { + val intent = Intent(Intent.ACTION_VIEW) + val permission = "Permission" + + broadcastSender.sendBroadcastAsUser(intent, UserHandle.ALL, permission, 12) + + runExecutorAssertingWakelock { + verify(mockContext).sendBroadcastAsUser(intent, UserHandle.ALL, permission, 12) + } + } + + @Test + fun sendCloseSystemDialogs_dispatchesWithWakelock() { + val intentCaptor = ArgumentCaptor.forClass(Intent::class.java) + + broadcastSender.closeSystemDialogs() + + runExecutorAssertingWakelock { + verify(mockContext).sendBroadcast(intentCaptor.capture()) + assertThat(intentCaptor.value.action).isEqualTo(Intent.ACTION_CLOSE_SYSTEM_DIALOGS) + } + } + + private fun runExecutorAssertingWakelock(verification: () -> Unit) { + assertThat(wakeLock.isHeld).isTrue() + executor.runAllReady() + verification.invoke() + assertThat(wakeLock.isHeld).isFalse() + } +} \ No newline at end of file