Fix ANR in TelephonyStatusControlSession
Feature.get() blocks on the main thread, which cause the ANR.
Cancel the job instead to fix.
Fix: 287702163
Test: Manually with MobileNetworkSettings
Test: atest TelephonyStatusControlSessionTest
Change-Id: Id873e56359dbf198c31686c2280c979294c95c3d
diff --git a/src/com/android/settings/network/telephony/AbstractMobileNetworkSettings.java b/src/com/android/settings/network/telephony/AbstractMobileNetworkSettings.java
index 245ac83..7addb59 100644
--- a/src/com/android/settings/network/telephony/AbstractMobileNetworkSettings.java
+++ b/src/com/android/settings/network/telephony/AbstractMobileNetworkSettings.java
@@ -18,7 +18,6 @@
import android.os.SystemClock;
import android.text.TextUtils;
-import android.util.Log;
import androidx.preference.Preference;
import androidx.preference.PreferenceScreen;
@@ -66,8 +65,7 @@
TelephonyStatusControlSession setTelephonyAvailabilityStatus(
Collection<AbstractPreferenceController> listOfPrefControllers) {
- return (new TelephonyStatusControlSession.Builder(listOfPrefControllers))
- .build();
+ return new TelephonyStatusControlSession(listOfPrefControllers, getLifecycle());
}
@Override
diff --git a/src/com/android/settings/network/telephony/TelephonyStatusControlSession.java b/src/com/android/settings/network/telephony/TelephonyStatusControlSession.java
deleted file mode 100644
index 3716f1f..0000000
--- a/src/com/android/settings/network/telephony/TelephonyStatusControlSession.java
+++ /dev/null
@@ -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();
- });
- }
-}
diff --git a/src/com/android/settings/network/telephony/TelephonyStatusControlSession.kt b/src/com/android/settings/network/telephony/TelephonyStatusControlSession.kt
new file mode 100644
index 0000000..0e63c8c
--- /dev/null
+++ b/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"
+ }
+}
diff --git a/tests/spa_unit/src/com/android/settings/network/telephony/TelephonyStatusControlSessionTest.kt b/tests/spa_unit/src/com/android/settings/network/telephony/TelephonyStatusControlSessionTest.kt
new file mode 100644
index 0000000..7e6a91b
--- /dev/null
+++ b/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
+ }
+ }
+}