Merge "Fix ANR in TelephonyStatusControlSession" into udc-qpr-dev am: 99b29e068e

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

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

src/com/android/settings/network/telephony/AbstractMobileNetworkSettings.java

@@ -18,7 +18,6 @@ package com.android.settings.network.telephony;
 
 import android.os.SystemClock;
 import android.text.TextUtils;
-import android.util.Log;
 
 import androidx.preference.Preference;
 import androidx.preference.PreferenceScreen;
@@ -66,8 +65,7 @@ abstract class AbstractMobileNetworkSettings extends RestrictedDashboardFragment
 
     TelephonyStatusControlSession setTelephonyAvailabilityStatus(
             Collection<AbstractPreferenceController> listOfPrefControllers) {
-        return (new TelephonyStatusControlSession.Builder(listOfPrefControllers))
+        return new TelephonyStatusControlSession(listOfPrefControllers, getLifecycle());
-                .build();
     }
 
     @Override

src/com/android/settings/network/telephony/TelephonyStatusControlSession.java

@@ -1,117 +0,0 @@
-/*
- * Copyright (C) 2020 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.network.telephony;
-
-import android.util.Log;
-
-import com.android.settings.core.BasePreferenceController;
-import com.android.settingslib.core.AbstractPreferenceController;
-import com.android.settingslib.utils.ThreadUtils;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
-
-/**
- * Session for controlling the status of TelephonyPreferenceController(s).
- *
- * Within this session, result of {@link BasePreferenceController#availabilityStatus()}
- * would be under control.
- */
-public class TelephonyStatusControlSession implements AutoCloseable {
-
-    private static final String LOG_TAG = "TelephonyStatusControlSS";
-
-    private Collection<AbstractPreferenceController> mControllers;
-    private Collection<Future<Boolean>> mResult = new ArrayList<>();
-
-    /**
-     * Buider of session
-     */
-    public static class Builder {
-        private Collection<AbstractPreferenceController> mControllers;
-
-        /**
-         * Constructor
-         *
-         * @param controllers is a collection of {@link AbstractPreferenceController}
-         *        which would have {@link BasePreferenceController#availabilityStatus()}
-         *        under control within this session.
-         */
-        public Builder(Collection<AbstractPreferenceController> controllers) {
-            mControllers = controllers;
-        }
-
-        /**
-         * Method to build this session.
-         * @return {@link TelephonyStatusControlSession} session been setup.
-         */
-        public TelephonyStatusControlSession build() {
-            return new TelephonyStatusControlSession(mControllers);
-        }
-    }
-
-    private TelephonyStatusControlSession(Collection<AbstractPreferenceController> controllers) {
-        mControllers = controllers;
-        controllers.forEach(prefCtrl -> mResult
-                .add(ThreadUtils.postOnBackgroundThread(() -> setupAvailabilityStatus(prefCtrl))));
-
-    }
-
-    /**
-     * Close the session.
-     *
-     * No longer control the status.
-     */
-    public void close() {
-        //check the background thread is finished then unset the status of availability.
-
-        for (Future<Boolean> result : mResult) {
-            try {
-                result.get();
-            } catch (ExecutionException | InterruptedException exception) {
-                Log.e(LOG_TAG, "setup availability status failed!", exception);
-            }
-        }
-        unsetAvailabilityStatus(mControllers);
-    }
-
-    private Boolean setupAvailabilityStatus(AbstractPreferenceController controller) {
-        try {
-            if (controller instanceof TelephonyAvailabilityHandler) {
-                int status = ((BasePreferenceController) controller)
-                        .getAvailabilityStatus();
-                ((TelephonyAvailabilityHandler) controller).setAvailabilityStatus(status);
-            }
-            return true;
-        } catch (Exception exception) {
-            Log.e(LOG_TAG, "Setup availability status failed!", exception);
-            return false;
-        }
-    }
-
-    private void unsetAvailabilityStatus(
-            Collection<AbstractPreferenceController> controllerLists) {
-        controllerLists.stream()
-                .filter(controller -> controller instanceof TelephonyAvailabilityHandler)
-                .map(TelephonyAvailabilityHandler.class::cast)
-                .forEach(controller -> {
-                    controller.unsetAvailabilityStatus();
-                });
-    }
-}

src/com/android/settings/network/telephony/TelephonyStatusControlSession.kt

@@ -0,0 +1,86 @@
+/*
+ * 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.network.telephony
+
+import android.util.Log
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.coroutineScope
+import com.android.settings.core.BasePreferenceController
+import com.android.settingslib.core.AbstractPreferenceController
+import com.google.common.collect.Sets
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.yield
+
+/**
+ * Session for controlling the status of TelephonyPreferenceController(s).
+ *
+ * Within this session, result of [BasePreferenceController.getAvailabilityStatus]
+ * would be under control.
+ */
+class TelephonyStatusControlSession(
+    private val controllers: Collection<AbstractPreferenceController>,
+    lifecycle: Lifecycle,
+) : AutoCloseable {
+    private var job: Job? = null
+    private val controllerSet = Sets.newConcurrentHashSet<TelephonyAvailabilityHandler>()
+
+    init {
+        job = lifecycle.coroutineScope.launch(Dispatchers.Default) {
+            for (controller in controllers) {
+                launch {
+                    setupAvailabilityStatus(controller)
+                }
+            }
+        }
+    }
+
+    /**
+     * Close the session.
+     *
+     * No longer control the status.
+     */
+    override fun close() {
+        job?.cancel()
+        unsetAvailabilityStatus()
+    }
+
+    private suspend fun setupAvailabilityStatus(controller: AbstractPreferenceController): Boolean =
+        try {
+            if (controller is TelephonyAvailabilityHandler) {
+                val status = (controller as BasePreferenceController).availabilityStatus
+                yield() // prompt cancellation guarantee
+                if (controllerSet.add(controller)) {
+                    controller.setAvailabilityStatus(status)
+                }
+            }
+            true
+        } catch (exception: Exception) {
+            Log.e(LOG_TAG, "Setup availability status failed!", exception)
+            false
+        }
+
+    private fun unsetAvailabilityStatus() {
+        for (controller in controllerSet) {
+            controller.unsetAvailabilityStatus()
+        }
+    }
+
+    companion object {
+        private const val LOG_TAG = "TelephonyStatusControlSS"
+    }
+}

tests/spa_unit/src/com/android/settings/network/telephony/TelephonyStatusControlSessionTest.kt

@@ -0,0 +1,81 @@
+/*
+ * 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.network.telephony
+
+import android.content.Context
+import androidx.lifecycle.testing.TestLifecycleOwner
+import androidx.test.core.app.ApplicationProvider
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.android.settings.core.BasePreferenceController
+import com.android.settingslib.spa.testutils.waitUntil
+import com.google.common.truth.Truth.assertThat
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.runTest
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@OptIn(ExperimentalCoroutinesApi::class)
+@RunWith(AndroidJUnit4::class)
+class TelephonyStatusControlSessionTest {
+    private val context: Context = ApplicationProvider.getApplicationContext()
+
+    @Test
+    fun init() = runTest {
+        val controller = TestController(context)
+
+        val session = TelephonyStatusControlSession(
+            controllers = listOf(controller),
+            lifecycle = TestLifecycleOwner().lifecycle,
+        )
+
+        waitUntil { controller.availabilityStatus == STATUS }
+        session.close()
+    }
+
+    @Test
+    fun close() = runTest {
+        val controller = TestController(context)
+
+        val session = TelephonyStatusControlSession(
+            controllers = listOf(controller),
+            lifecycle = TestLifecycleOwner().lifecycle,
+        )
+        session.close()
+
+        assertThat(controller.availabilityStatus).isNull()
+    }
+
+    private companion object {
+        const val KEY = "key"
+        const val STATUS = BasePreferenceController.AVAILABLE
+    }
+
+    private class TestController(context: Context) : BasePreferenceController(context, KEY),
+        TelephonyAvailabilityHandler {
+
+        var availabilityStatus: Int? = null
+        override fun getAvailabilityStatus(): Int = STATUS
+
+        override fun setAvailabilityStatus(status: Int) {
+            availabilityStatus = status
+        }
+
+        override fun unsetAvailabilityStatus() {
+            availabilityStatus = null
+        }
+    }
+}