Merge "Fix dialog leak in RequestPermissionActivity" into udc-dev am: 5b18837a49

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/apps/Settings/+/23012490

Change-Id: I26a1d41728d164f5a9d226d701421a5bed242792
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

src/com/android/settings/bluetooth/RequestPermissionActivity.java

@@ -72,6 +72,7 @@ public class RequestPermissionActivity extends Activity implements
     private int mRequest;
 
     private AlertDialog mDialog;
+    private AlertDialog mRequestDialog;
 
     private BroadcastReceiver mReceiver;
 
@@ -96,12 +97,12 @@ public class RequestPermissionActivity extends Activity implements
         if (mRequest == REQUEST_DISABLE) {
             switch (btState) {
                 case BluetoothAdapter.STATE_OFF:
-                case BluetoothAdapter.STATE_TURNING_OFF: {
+                case BluetoothAdapter.STATE_TURNING_OFF:
                     proceedAndFinish();
-                } break;
+                    break;
-
                 case BluetoothAdapter.STATE_ON:
-                case BluetoothAdapter.STATE_TURNING_ON: {
+                case BluetoothAdapter.STATE_TURNING_ON:
+                    mRequestDialog =
                             RequestPermissionHelper.INSTANCE.requestDisable(this, mAppLabel,
                                     () -> {
                                         onDisableConfirmed();
@@ -111,18 +112,20 @@ public class RequestPermissionActivity extends Activity implements
                                         cancelAndFinish();
                                         return Unit.INSTANCE;
                                     });
-                } break;
+                    if (mRequestDialog != null) {
-
+                        mRequestDialog.show();
-                default: {
+                    }
+                    break;
+                default:
                     Log.e(TAG, "Unknown adapter state: " + btState);
                     cancelAndFinish();
-                } break;
+                    break;
             }
         } else {
             switch (btState) {
                 case BluetoothAdapter.STATE_OFF:
                 case BluetoothAdapter.STATE_TURNING_OFF:
-                case BluetoothAdapter.STATE_TURNING_ON: {
+                case BluetoothAdapter.STATE_TURNING_ON:
                     /*
                      * Strictly speaking STATE_TURNING_ON belong with STATE_ON;
                      * however, BT may not be ready when the user clicks yes and we
@@ -131,7 +134,8 @@ public class RequestPermissionActivity extends Activity implements
                      * case via the broadcast receiver.
                      */
 
-                    // Start the helper activity to ask the user about enabling bt AND discovery
+                    // Show the helper dialog to ask the user about enabling bt AND discovery
+                    mRequestDialog =
                             RequestPermissionHelper.INSTANCE.requestEnable(this, mAppLabel,
                                     mRequest == REQUEST_ENABLE_DISCOVERABLE ? mTimeout : -1,
                                     () -> {
@@ -142,9 +146,11 @@ public class RequestPermissionActivity extends Activity implements
                                         cancelAndFinish();
                                         return Unit.INSTANCE;
                                     });
-                } break;
+                    if (mRequestDialog != null) {
-
+                        mRequestDialog.show();
-                case BluetoothAdapter.STATE_ON: {
+                    }
+                    break;
+                case BluetoothAdapter.STATE_ON:
                     if (mRequest == REQUEST_ENABLE) {
                         // Nothing to do. Already enabled.
                         proceedAndFinish();
@@ -152,12 +158,11 @@ public class RequestPermissionActivity extends Activity implements
                         // Ask the user about enabling discovery mode
                         createDialog();
                     }
-                } break;
+                    break;
-
+                default:
-                default: {
                     Log.e(TAG, "Unknown adapter state: " + btState);
                     cancelAndFinish();
-                } break;
+                    break;
             }
         }
     }
@@ -275,10 +280,6 @@ public class RequestPermissionActivity extends Activity implements
             }
         }
 
-        if (mDialog != null) {
-            mDialog.dismiss();
-        }
-
         setResult(returnCode);
         finish();
     }
@@ -365,6 +366,14 @@ public class RequestPermissionActivity extends Activity implements
             unregisterReceiver(mReceiver);
             mReceiver = null;
         }
+        if (mDialog != null && mDialog.isShowing()) {
+            mDialog.dismiss();
+            mDialog = null;
+        }
+        if (mRequestDialog != null && mRequestDialog.isShowing()) {
+            mRequestDialog.dismiss();
+            mRequestDialog = null;
+        }
     }
 
     @Override

src/com/android/settings/bluetooth/RequestPermissionHelper.kt

@@ -30,20 +30,20 @@ object RequestPermissionHelper {
         timeout: Int,
         onAllow: () -> Unit,
         onDeny: () -> Unit,
-    ) {
+    ): AlertDialog? {
         if (context.resources.getBoolean(R.bool.auto_confirm_bluetooth_activation_dialog)) {
             // Don't even show the dialog if configured this way
             onAllow()
-            return
+            return null
         }
-        AlertDialog.Builder(context).apply {
+        return AlertDialog.Builder(context).apply {
             setMessage(context.getEnableMessage(timeout, appLabel))
             setPositiveButton(R.string.allow) { _, _ ->
                 if (context.isDisallowBluetooth()) onDeny() else onAllow()
             }
             setNegativeButton(R.string.deny) { _, _ -> onDeny() }
             setOnCancelListener { onDeny() }
-        }.show()
+        }.create()
     }
 
     fun requestDisable(
@@ -51,18 +51,18 @@ object RequestPermissionHelper {
         appLabel: CharSequence?,
         onAllow: () -> Unit,
         onDeny: () -> Unit,
-    ) {
+    ): AlertDialog? {
         if (context.resources.getBoolean(R.bool.auto_confirm_bluetooth_activation_dialog)) {
             // Don't even show the dialog if configured this way
             onAllow()
-            return
+            return null
         }
-        AlertDialog.Builder(context).apply {
+        return AlertDialog.Builder(context).apply {
             setMessage(context.getDisableMessage(appLabel))
             setPositiveButton(R.string.allow) { _, _ -> onAllow() }
             setNegativeButton(R.string.deny) { _, _ -> onDeny() }
             setOnCancelListener { onDeny() }
-        }.show()
+        }.create()
     }
 }
 

tests/robotests/src/com/android/settings/bluetooth/RequestPermissionActivityTest.kt

@@ -0,0 +1,102 @@
+/*
+ * Copyright (C) 2023 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.settings.bluetooth
+
+import android.bluetooth.BluetoothAdapter
+import android.content.Intent
+import com.android.settings.testutils.shadow.ShadowAlertDialogCompat
+import com.google.common.truth.Truth.assertThat
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.android.controller.ActivityController
+import org.robolectric.annotation.Config
+import org.robolectric.shadow.api.Shadow
+import org.robolectric.shadows.ShadowBluetoothAdapter
+
+@RunWith(RobolectricTestRunner::class)
+@Config(shadows = [ShadowAlertDialogCompat::class, ShadowBluetoothAdapter::class])
+class RequestPermissionActivityTest {
+    private lateinit var activityController: ActivityController<RequestPermissionActivity>
+    private lateinit var bluetoothAdapter: ShadowBluetoothAdapter
+
+    @Before
+    fun setUp() {
+        bluetoothAdapter = Shadow.extract(BluetoothAdapter.getDefaultAdapter())
+    }
+
+    @After
+    fun tearDown() {
+        activityController.pause().stop().destroy()
+        ShadowAlertDialogCompat.reset()
+    }
+
+    @Test
+    fun requestEnable_whenBluetoothIsOff_showConfirmDialog() {
+        bluetoothAdapter.setState(BluetoothAdapter.STATE_OFF)
+
+        createActivity(action = BluetoothAdapter.ACTION_REQUEST_ENABLE)
+
+        val dialog = ShadowAlertDialogCompat.getLatestAlertDialog()
+        val shadowDialog = ShadowAlertDialogCompat.shadowOf(dialog)
+        assertThat(shadowDialog.message.toString())
+            .isEqualTo("An app wants to turn on Bluetooth")
+    }
+
+    @Test
+    fun requestEnable_whenBluetoothIsOn_doNothing() {
+        bluetoothAdapter.setState(BluetoothAdapter.STATE_ON)
+
+        createActivity(action = BluetoothAdapter.ACTION_REQUEST_ENABLE)
+
+        val dialog = ShadowAlertDialogCompat.getLatestAlertDialog()
+        assertThat(dialog).isNull()
+    }
+
+    @Test
+    fun requestDisable_whenBluetoothIsOff_doNothing() {
+        bluetoothAdapter.setState(BluetoothAdapter.STATE_OFF)
+
+        createActivity(action = BluetoothAdapter.ACTION_REQUEST_DISABLE)
+
+        val dialog = ShadowAlertDialogCompat.getLatestAlertDialog()
+        assertThat(dialog).isNull()
+    }
+
+    @Test
+    fun requestDisable_whenBluetoothIsOn_showConfirmDialog() {
+        bluetoothAdapter.setState(BluetoothAdapter.STATE_ON)
+
+        createActivity(action = BluetoothAdapter.ACTION_REQUEST_DISABLE)
+
+        val dialog = ShadowAlertDialogCompat.getLatestAlertDialog()
+        val shadowDialog = ShadowAlertDialogCompat.shadowOf(dialog)
+        assertThat(shadowDialog.message.toString())
+            .isEqualTo("An app wants to turn off Bluetooth")
+    }
+
+    private fun createActivity(action: String) {
+        activityController =
+            ActivityController.of(RequestPermissionActivity(), Intent(action)).apply {
+                create()
+                start()
+                postCreate(null)
+                resume()
+            }
+    }
+}

tests/robotests/src/com/android/settings/bluetooth/RequestPermissionHelperTest.kt

@@ -49,13 +49,14 @@ class RequestPermissionHelperTest {
     @Test
     fun requestEnable_withAppLabelAndNoTimeout_hasCorrectMessage() {
         val activity = activityController.get()
+
         RequestPermissionHelper.requestEnable(
             context = activity,
             appLabel = "App Label",
             timeout = -1,
             onAllow = {},
             onDeny = {},
-        )
+        )!!.show()
 
         assertLatestMessageIs("App Label wants to turn on Bluetooth")
     }
@@ -63,13 +64,14 @@ class RequestPermissionHelperTest {
     @Test
     fun requestEnable_withAppLabelAndZeroTimeout_hasCorrectMessage() {
         val activity = activityController.get()
+
         RequestPermissionHelper.requestEnable(
             context = activity,
             appLabel = "App Label",
             timeout = 0,
             onAllow = {},
             onDeny = {},
-        )
+        )!!.show()
 
         assertLatestMessageIs(
             "App Label wants to turn on Bluetooth and make your phone visible to other devices. " +
@@ -80,13 +82,14 @@ class RequestPermissionHelperTest {
     @Test
     fun requestEnable_withAppLabelAndNormalTimeout_hasCorrectMessage() {
         val activity = activityController.get()
+
         RequestPermissionHelper.requestEnable(
             context = activity,
             appLabel = "App Label",
             timeout = 120,
             onAllow = {},
             onDeny = {},
-        )
+        )!!.show()
 
         assertLatestMessageIs(
             "App Label wants to turn on Bluetooth and make your phone visible to other devices " +
@@ -97,13 +100,14 @@ class RequestPermissionHelperTest {
     @Test
     fun requestEnable_withNoAppLabelAndNoTimeout_hasCorrectMessage() {
         val activity = activityController.get()
+
         RequestPermissionHelper.requestEnable(
             context = activity,
             appLabel = null,
             timeout = -1,
             onAllow = {},
             onDeny = {},
-        )
+        )!!.show()
 
         assertLatestMessageIs("An app wants to turn on Bluetooth")
     }
@@ -111,13 +115,14 @@ class RequestPermissionHelperTest {
     @Test
     fun requestEnable_withNoAppLabelAndZeroTimeout_hasCorrectMessage() {
         val activity = activityController.get()
+
         RequestPermissionHelper.requestEnable(
             context = activity,
             appLabel = null,
             timeout = 0,
             onAllow = {},
             onDeny = {},
-        )
+        )!!.show()
 
         assertLatestMessageIs(
             "An app wants to turn on Bluetooth and make your phone visible to other devices. " +
@@ -128,13 +133,14 @@ class RequestPermissionHelperTest {
     @Test
     fun requestEnable_withNoAppLabelAndNormalTimeout_hasCorrectMessage() {
         val activity = activityController.get()
+
         RequestPermissionHelper.requestEnable(
             context = activity,
             appLabel = null,
             timeout = 120,
             onAllow = {},
             onDeny = {},
-        )
+        )!!.show()
 
         assertLatestMessageIs(
             "An app wants to turn on Bluetooth and make your phone visible to other devices for " +
@@ -177,12 +183,13 @@ class RequestPermissionHelperTest {
     @Test
     fun requestDisable_withAppLabel_hasCorrectMessage() {
         val activity = activityController.get()
+
         RequestPermissionHelper.requestDisable(
             context = activity,
             appLabel = "App Label",
             onAllow = {},
             onDeny = {},
-        )
+        )!!.show()
 
         assertLatestMessageIs("App Label wants to turn off Bluetooth")
     }
@@ -190,12 +197,13 @@ class RequestPermissionHelperTest {
     @Test
     fun requestDisable_withNoAppLabel_hasCorrectMessage() {
         val activity = activityController.get()
+
         RequestPermissionHelper.requestDisable(
             context = activity,
             appLabel = null,
             onAllow = {},
             onDeny = {},
-        )
+        )!!.show()
 
         assertLatestMessageIs("An app wants to turn off Bluetooth")
     }