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

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<Int> { response ->
+        val dialogResponse = SingleShotConsumer<Int> { response ->
             if (response == ADD_TILE) {
                 addTile(componentName)
             }
@@ -127,7 +133,7 @@ class TileServiceRequestController constructor(
 
     private fun createDialog(
         tileData: TileRequestDialog.TileData,
-        responseHandler: Consumer<Int>
+        responseHandler: SingleShotConsumer<Int>
     ): 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<T>(private val consumer: Consumer<T>) : Consumer<T> {
+        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,

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<Int> {
         var lastAccepted: Int? = null
             private set
+
+        var timesCalled = 0
+            private set
+
         override fun accept(t: Int) {
             lastAccepted = t
+            timesCalled++
         }
 
         override fun onTileRequest(r: Int) {