From 3cf1b970d40da9fdc9f425f03d79c570107f80c3 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Fri, 4 Feb 2022 10:47:56 -0500 Subject: [PATCH] Fix tile request dialog dismiss When the dialog is dismissed without being cancelled (for example, by going home or locking the phone), make sure to also send a dismissed message. As dismissed is always called after cancelled or click in a button, make sure that this message is only sent once. Also, check for RemoteException when communicating with the Binder interface. Test: atest TileServiceRequestControllerTest Test: manual with test app Test: CtsVerifier Fixes: 217887724 Change-Id: I286c24ae743aede158214880679abd07c3db3ff6 --- .../external/TileServiceRequestController.kt | 26 +++++- .../TileServiceRequestControllerTest.kt | 79 +++++++++++++++++++ 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceRequestController.kt b/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceRequestController.kt index 73d6b971f7850..237b66e79ee58 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceRequestController.kt +++ b/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceRequestController.kt @@ -21,6 +21,7 @@ import android.app.StatusBarManager import android.content.ComponentName import android.content.DialogInterface import android.graphics.drawable.Icon +import android.os.RemoteException import android.util.Log import androidx.annotation.VisibleForTesting import com.android.internal.statusbar.IAddTileResultCallback @@ -32,6 +33,7 @@ import com.android.systemui.statusbar.phone.SystemUIDialog import com.android.systemui.R import com.android.systemui.statusbar.CommandQueue import java.io.PrintWriter +import java.util.concurrent.atomic.AtomicBoolean import java.util.function.Consumer import javax.inject.Inject @@ -67,7 +69,11 @@ class TileServiceRequestController constructor( callback: IAddTileResultCallback ) { requestTileAdd(componentName, appName, label, icon) { - callback.onTileRequest(it) + try { + callback.onTileRequest(it) + } catch (e: RemoteException) { + Log.e(TAG, "Couldn't respond to request", e) + } } } @@ -105,7 +111,7 @@ class TileServiceRequestController constructor( eventLogger.logTileAlreadyAdded(packageName, instanceId) return } - val dialogResponse = Consumer { response -> + val dialogResponse = SingleShotConsumer { response -> if (response == ADD_TILE) { addTile(componentName) } @@ -127,7 +133,7 @@ class TileServiceRequestController constructor( private fun createDialog( tileData: TileRequestDialog.TileData, - responseHandler: Consumer + responseHandler: SingleShotConsumer ): SystemUIDialog { val dialogClickListener = DialogInterface.OnClickListener { _, which -> if (which == Dialog.BUTTON_POSITIVE) { @@ -141,6 +147,10 @@ class TileServiceRequestController constructor( setShowForAllUsers(true) setCanceledOnTouchOutside(true) setOnCancelListener { responseHandler.accept(DISMISSED) } + // We want this in case the dialog is dismissed without it being cancelled (for example + // by going home or locking the device). We use a SingleShotConsumer so the response + // is only sent once, with the first value. + setOnDismissListener { responseHandler.accept(DISMISSED) } setPositiveButton(R.string.qs_tile_request_dialog_add, dialogClickListener) setNegativeButton(R.string.qs_tile_request_dialog_not_add, dialogClickListener) } @@ -169,6 +179,16 @@ class TileServiceRequestController constructor( } } + private class SingleShotConsumer(private val consumer: Consumer) : Consumer { + private val dispatched = AtomicBoolean(false) + + override fun accept(t: T) { + if (dispatched.compareAndSet(false, true)) { + consumer.accept(t) + } + } + } + @SysUISingleton class Builder @Inject constructor( private val commandQueue: CommandQueue, diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServiceRequestControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServiceRequestControllerTest.kt index a1c60a648de91..bdfbca47e5692 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServiceRequestControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServiceRequestControllerTest.kt @@ -20,6 +20,7 @@ import android.app.StatusBarManager import android.content.ComponentName import android.content.DialogInterface import android.graphics.drawable.Icon +import android.os.RemoteException import android.testing.AndroidTestingRunner import androidx.test.filters.SmallTest import com.android.internal.logging.InstanceId @@ -275,11 +276,89 @@ class TileServiceRequestControllerTest : SysuiTestCase() { assertThat(c.lastAccepted).isEqualTo(TileServiceRequestController.TILE_ALREADY_ADDED) } + @Test + fun interfaceThrowsRemoteException_doesntCrash() { + val cancelListenerCaptor = + ArgumentCaptor.forClass(DialogInterface.OnCancelListener::class.java) + val captor = ArgumentCaptor.forClass(CommandQueue.Callbacks::class.java) + verify(commandQueue, atLeastOnce()).addCallback(capture(captor)) + + val callback = object : IAddTileResultCallback.Stub() { + override fun onTileRequest(p0: Int) { + throw RemoteException() + } + } + captor.value.requestAddTile(TEST_COMPONENT, TEST_APP_NAME, TEST_LABEL, icon, callback) + verify(tileRequestDialog).setOnCancelListener(capture(cancelListenerCaptor)) + + cancelListenerCaptor.value.onCancel(tileRequestDialog) + } + + @Test + fun testDismissDialogResponse() { + val dismissListenerCaptor = + ArgumentCaptor.forClass(DialogInterface.OnDismissListener::class.java) + + val callback = Callback() + controller.requestTileAdd(TEST_COMPONENT, TEST_APP_NAME, TEST_LABEL, icon, callback) + verify(tileRequestDialog).setOnDismissListener(capture(dismissListenerCaptor)) + + dismissListenerCaptor.value.onDismiss(tileRequestDialog) + assertThat(callback.lastAccepted).isEqualTo(TileServiceRequestController.DISMISSED) + } + + @Test + fun addTileAndThenDismissSendsOnlyAddTile() { + // After clicking, the dialog is dismissed. This tests that only one response + // is sent (the first one) + val dismissListenerCaptor = + ArgumentCaptor.forClass(DialogInterface.OnDismissListener::class.java) + val clickListenerCaptor = + ArgumentCaptor.forClass(DialogInterface.OnClickListener::class.java) + + val callback = Callback() + controller.requestTileAdd(TEST_COMPONENT, TEST_APP_NAME, TEST_LABEL, icon, callback) + verify(tileRequestDialog).setPositiveButton(anyInt(), capture(clickListenerCaptor)) + verify(tileRequestDialog).setOnDismissListener(capture(dismissListenerCaptor)) + + clickListenerCaptor.value.onClick(tileRequestDialog, DialogInterface.BUTTON_POSITIVE) + dismissListenerCaptor.value.onDismiss(tileRequestDialog) + + assertThat(callback.lastAccepted).isEqualTo(TileServiceRequestController.ADD_TILE) + assertThat(callback.timesCalled).isEqualTo(1) + } + + @Test + fun cancelAndThenDismissSendsOnlyOnce() { + // After cancelling, the dialog is dismissed. This tests that only one response + // is sent. + val dismissListenerCaptor = + ArgumentCaptor.forClass(DialogInterface.OnDismissListener::class.java) + val cancelListenerCaptor = + ArgumentCaptor.forClass(DialogInterface.OnCancelListener::class.java) + + val callback = Callback() + controller.requestTileAdd(TEST_COMPONENT, TEST_APP_NAME, TEST_LABEL, icon, callback) + verify(tileRequestDialog).setOnCancelListener(capture(cancelListenerCaptor)) + verify(tileRequestDialog).setOnDismissListener(capture(dismissListenerCaptor)) + + cancelListenerCaptor.value.onCancel(tileRequestDialog) + dismissListenerCaptor.value.onDismiss(tileRequestDialog) + + assertThat(callback.lastAccepted).isEqualTo(TileServiceRequestController.DISMISSED) + assertThat(callback.timesCalled).isEqualTo(1) + } + private class Callback : IAddTileResultCallback.Stub(), Consumer { var lastAccepted: Int? = null private set + + var timesCalled = 0 + private set + override fun accept(t: Int) { lastAccepted = t + timesCalled++ } override fun onTileRequest(r: Int) {