Fix empty network scan result

Settings use TelephonyScanManager.NetworkScanCallback to scan, and its
onComplete() could happens before onResults().

We will change Settings to fix.

Fix: 338986191
Test: manual - on NetworkSelectSettings
Test: unit test
Change-Id: If41d957f916a99eacc1becb6b460e58722a4dca7
Merged-In: If41d957f916a99eacc1becb6b460e58722a4dca7

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

@@ -50,16 +50,13 @@ import com.android.internal.telephony.flags.Flags;
 import com.android.settings.R;
 import com.android.settings.dashboard.DashboardFragment;
 import com.android.settings.network.telephony.scan.NetworkScanRepository;
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanCellInfos;
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanComplete;
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanError;
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanResult;
 import com.android.settings.overlay.FeatureFactory;
 import com.android.settingslib.core.instrumentation.MetricsFeatureProvider;
 import com.android.settingslib.utils.ThreadUtils;
 
+import com.google.common.collect.ImmutableList;
+
 import kotlin.Unit;
-import kotlin.jvm.functions.Function1;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -87,7 +84,8 @@ public class NetworkSelectSettings extends DashboardFragment {
     private View mProgressHeader;
     private Preference mStatusMessagePreference;
     @VisibleForTesting
-    List<CellInfo> mCellInfoList;
+    @NonNull
+    List<CellInfo> mCellInfoList = ImmutableList.of();
     private int mSubId = SubscriptionManager.INVALID_SUBSCRIPTION_ID;
     private TelephonyManager mTelephonyManager;
     private SatelliteManager mSatelliteManager;
@@ -100,7 +98,6 @@ public class NetworkSelectSettings extends DashboardFragment {
     private AtomicBoolean mShouldFilterOutSatellitePlmn = new AtomicBoolean();
 
     private NetworkScanRepository mNetworkScanRepository;
-    private boolean mUpdateScanResult = false;
 
     @Override
     public void onCreate(Bundle icicle) {
@@ -208,40 +205,14 @@ public class NetworkSelectSettings extends DashboardFragment {
     }
 
     private void launchNetworkScan() {
-        mNetworkScanRepository.launchNetworkScan(getViewLifecycleOwner(), new Function1<>() {
-            @Override
-            public Unit invoke(@NonNull NetworkScanResult networkScanResult) {
-                if (!mUpdateScanResult) {
-                    // Not update UI if not in scan mode.
-                    return Unit.INSTANCE;
-                }
-                if (networkScanResult instanceof NetworkScanCellInfos networkScanCellInfos) {
-                    scanResultHandler(networkScanCellInfos.getCellInfos());
-                    return Unit.INSTANCE;
-                }
-                if (!isPreferenceScreenEnabled()) {
-                    clearPreferenceSummary();
-                    enablePreferenceScreen(true);
-                } else if (networkScanResult instanceof NetworkScanComplete
-                        && mCellInfoList == null) {
-                    // In case the scan timeout before getting any results
-                    addMessagePreference(R.string.empty_networks_list);
-                } else if (networkScanResult instanceof NetworkScanError) {
-                    addMessagePreference(R.string.network_query_error);
-                }
-
-                return Unit.INSTANCE;
-            }
-        });
-    }
-
-    @Override
-    public void onStart() {
-        super.onStart();
-
-        updateForbiddenPlmns();
         setProgressBarVisible(true);
-        mUpdateScanResult = true;
+        mNetworkScanRepository.launchNetworkScan(getViewLifecycleOwner(), (networkScanResult) -> {
+            if (isPreferenceScreenEnabled()) {
+                scanResultHandler(networkScanResult);
+            }
+
+            return Unit.INSTANCE;
+        });
     }
 
     /**
@@ -267,8 +238,6 @@ public class NetworkSelectSettings extends DashboardFragment {
             return false;
         }
 
-        mUpdateScanResult = false;
-
         // Refresh the last selected item in case users reselect network.
         clearPreferenceSummary();
         if (mSelectedPreference != null) {
@@ -373,27 +342,19 @@ public class NetworkSelectSettings extends DashboardFragment {
         }
     }
 
-    @Keep
     @VisibleForTesting
-    protected void scanResultHandler(List<CellInfo> results) {
+    protected void scanResultHandler(NetworkScanRepository.NetworkScanResult results) {
-        mCellInfoList = filterOutSatellitePlmn(results);
+        mCellInfoList = filterOutSatellitePlmn(results.getCellInfos());
         Log.d(TAG, "CellInfoList: " + CellInfoUtil.cellInfoListToString(mCellInfoList));
-        if (mCellInfoList != null && mCellInfoList.size() != 0) {
+        updateAllPreferenceCategory();
-            final NetworkOperatorPreference connectedPref = updateAllPreferenceCategory();
+        NetworkScanRepository.NetworkScanState state = results.getState();
-            if (connectedPref != null) {
+        if (state == NetworkScanRepository.NetworkScanState.ERROR) {
-                // update selected preference instance into connected preference
+            addMessagePreference(R.string.network_query_error);
-                if (mSelectedPreference != null) {
+        } else if (mCellInfoList.isEmpty()) {
-                    mSelectedPreference = connectedPref;
-                }
-            } else if (!isPreferenceScreenEnabled()) {
-                mSelectedPreference.setSummary(R.string.network_connecting);
-            }
-            enablePreferenceScreen(true);
-        } else if (isPreferenceScreenEnabled()) {
             addMessagePreference(R.string.empty_networks_list);
-            // keep showing progress bar, it will be stopped when error or completed
-            setProgressBarVisible(true);
         }
+        // keep showing progress bar, it will be stopped when error or completed
+        setProgressBarVisible(state == NetworkScanRepository.NetworkScanState.ACTIVE);
     }
 
     @Keep
@@ -410,11 +371,8 @@ public class NetworkSelectSettings extends DashboardFragment {
 
     /**
      * Update the content of network operators list.
-     *
-     * @return preference which shows connected
      */
-    @Nullable
+    private void updateAllPreferenceCategory() {
-    private NetworkOperatorPreference updateAllPreferenceCategory() {
         int numberOfPreferences = mPreferenceCategory.getPreferenceCount();
 
         // remove unused preferences
@@ -425,7 +383,6 @@ public class NetworkSelectSettings extends DashboardFragment {
         }
 
         // update the content of preference
-        NetworkOperatorPreference connectedPref = null;
         for (int index = 0; index < mCellInfoList.size(); index++) {
             final CellInfo cellInfo = mCellInfoList.get(index);
 
@@ -450,23 +407,10 @@ public class NetworkSelectSettings extends DashboardFragment {
 
             if (mCellInfoList.get(index).isRegistered()) {
                 pref.setSummary(R.string.network_connected);
-                connectedPref = pref;
             } else {
                 pref.setSummary(null);
             }
         }
-
-        // update selected preference instance by index
-        for (int index = 0; index < mCellInfoList.size(); index++) {
-            final CellInfo cellInfo = mCellInfoList.get(index);
-
-            if ((mSelectedPreference != null) && mSelectedPreference.isSameCell(cellInfo)) {
-                mSelectedPreference = (NetworkOperatorPreference)
-                        (mPreferenceCategory.getPreference(index));
-            }
-        }
-
-        return connectedPref;
     }
 
     /**
@@ -535,13 +479,6 @@ public class NetworkSelectSettings extends DashboardFragment {
         }
     }
 
-    private boolean isProgressBarVisible() {
-        if (mProgressHeader == null) {
-            return false;
-        }
-        return (mProgressHeader.getVisibility() == View.VISIBLE);
-    }
-
     protected void setProgressBarVisible(boolean visible) {
         if (mProgressHeader != null) {
             mProgressHeader.setVisibility(visible ? View.VISIBLE : View.GONE);
@@ -549,7 +486,6 @@ public class NetworkSelectSettings extends DashboardFragment {
     }
 
     private void addMessagePreference(int messageId) {
-        setProgressBarVisible(false);
         mStatusMessagePreference.setTitle(messageId);
         mPreferenceCategory.removeAll();
         mPreferenceCategory.addPreference(mStatusMessagePreference);

src/com/android/settings/network/telephony/scan/NetworkScanRepository.kt

@@ -27,7 +27,6 @@ import android.telephony.TelephonyScanManager
 import android.util.Log
 import androidx.annotation.VisibleForTesting
 import androidx.lifecycle.LifecycleOwner
-import com.android.settings.network.telephony.CellInfoUtil
 import com.android.settings.network.telephony.CellInfoUtil.getNetworkTitle
 import com.android.settingslib.spa.framework.util.collectLatestWithLifecycle
 import kotlinx.coroutines.Dispatchers
@@ -35,14 +34,19 @@ import kotlinx.coroutines.asExecutor
 import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.callbackFlow
+import kotlinx.coroutines.flow.conflate
 import kotlinx.coroutines.flow.flowOn
+import kotlinx.coroutines.flow.onEach
 
 class NetworkScanRepository(context: Context, subId: Int) {
-    sealed interface NetworkScanResult
+    enum class NetworkScanState {
+        ACTIVE, COMPLETE, ERROR
+    }
 
-    data class NetworkScanCellInfos(val cellInfos: List<CellInfo>) : NetworkScanResult
+    data class NetworkScanResult(
-    data object NetworkScanComplete : NetworkScanResult
+        val state: NetworkScanState,
-    data class NetworkScanError(val error: Int) : NetworkScanResult
+        val cellInfos: List<CellInfo>,
+    )
 
     private val telephonyManager =
         context.getSystemService(TelephonyManager::class.java)!!.createForSubscriptionId(subId)
@@ -65,23 +69,29 @@ class NetworkScanRepository(context: Context, subId: Int) {
     }
 
     fun networkScanFlow(): Flow<NetworkScanResult> = callbackFlow {
+        var state = NetworkScanState.ACTIVE
+        var cellInfos: List<CellInfo> = emptyList()
+
         val callback = object : TelephonyScanManager.NetworkScanCallback() {
             override fun onResults(results: List<CellInfo>) {
-                val cellInfos = results.distinctBy { CellInfoScanKey(it) }
+                cellInfos = results.distinctBy { CellInfoScanKey(it) }
-                trySend(NetworkScanCellInfos(cellInfos))
+                sendResult()
-                Log.d(TAG, "CellInfoList: ${CellInfoUtil.cellInfoListToString(cellInfos)}")
             }
 
             override fun onComplete() {
-                trySend(NetworkScanComplete)
+                state = NetworkScanState.COMPLETE
-                close()
+                sendResult()
-                Log.d(TAG, "onComplete")
+                // Don't call close() here since onComplete() could happens before onResults()
             }
 
             override fun onError(error: Int) {
-                trySend(NetworkScanError(error))
+                state = NetworkScanState.ERROR
+                sendResult()
                 close()
-                Log.d(TAG, "onError: $error")
+            }
+
+            private fun sendResult() {
+                trySend(NetworkScanResult(state, cellInfos))
             }
         }
 
@@ -92,7 +102,7 @@ class NetworkScanRepository(context: Context, subId: Int) {
         )
 
         awaitClose { networkScan.stopScan() }
-    }.flowOn(Dispatchers.Default)
+    }.conflate().onEach { Log.d(TAG, "networkScanFlow: $it") }.flowOn(Dispatchers.Default)
 
     /** Create network scan for allowed network types. */
     private fun createNetworkScan(): NetworkScanRequest {

tests/spa_unit/src/com/android/settings/network/telephony/scan/NetworkScanRepositoryTest.kt

@@ -32,9 +32,6 @@ import android.telephony.TelephonyManager.NETWORK_CLASS_BITMASK_5G
 import android.telephony.TelephonyScanManager
 import androidx.test.core.app.ApplicationProvider
 import androidx.test.ext.junit.runners.AndroidJUnit4
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanCellInfos
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanComplete
-import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanError
 import com.android.settingslib.spa.testutils.firstWithTimeoutOrNull
 import com.android.settingslib.spa.testutils.toListWithTimeout
 import com.google.common.truth.Truth.assertThat
@@ -88,7 +85,12 @@ class NetworkScanRepositoryTest {
 
         callback?.onResults(cellInfos)
 
-        assertThat(listDeferred.await()).containsExactly(NetworkScanCellInfos(cellInfos))
+        assertThat(listDeferred.await()).containsExactly(
+            NetworkScanRepository.NetworkScanResult(
+                state = NetworkScanRepository.NetworkScanState.ACTIVE,
+                cellInfos = cellInfos,
+            )
+        )
     }
 
     @Test
@@ -100,7 +102,12 @@ class NetworkScanRepositoryTest {
 
         callback?.onComplete()
 
-        assertThat(listDeferred.await()).containsExactly(NetworkScanComplete)
+        assertThat(listDeferred.await()).containsExactly(
+            NetworkScanRepository.NetworkScanResult(
+                state = NetworkScanRepository.NetworkScanState.COMPLETE,
+                cellInfos = emptyList(),
+            )
+        )
     }
 
     @Test
@@ -112,7 +119,12 @@ class NetworkScanRepositoryTest {
 
         callback?.onError(1)
 
-        assertThat(listDeferred.await()).containsExactly(NetworkScanError(1))
+        assertThat(listDeferred.await()).containsExactly(
+            NetworkScanRepository.NetworkScanResult(
+                state = NetworkScanRepository.NetworkScanState.ERROR,
+                cellInfos = emptyList(),
+            )
+        )
     }
 
     @Test
@@ -133,12 +145,13 @@ class NetworkScanRepositoryTest {
         callback?.onResults(cellInfos)
 
         assertThat(listDeferred.await()).containsExactly(
-            NetworkScanCellInfos(
+            NetworkScanRepository.NetworkScanResult(
-                listOf(
+                state = NetworkScanRepository.NetworkScanState.ACTIVE,
+                cellInfos = listOf(
                     createCellInfoLte("123", false),
                     createCellInfoLte("124", true),
                     createCellInfoGsm("123", false),
-                )
+                ),
             )
         )
     }
@@ -162,8 +175,9 @@ class NetworkScanRepositoryTest {
         callback?.onResults(cellInfos)
 
         assertThat(listDeferred.await()).containsExactly(
-            NetworkScanCellInfos(
+            NetworkScanRepository.NetworkScanResult(
-                listOf(
+                state = NetworkScanRepository.NetworkScanState.ACTIVE,
+                cellInfos = listOf(
                     createCellInfoLte("123", false),
                     createCellInfoLte("123", true),
                     createCellInfoLte("124", false),

tests/unit/src/com/android/settings/network/telephony/NetworkSelectSettingsTest.java

@@ -44,8 +44,12 @@ import androidx.preference.PreferenceManager;
 import androidx.test.annotation.UiThreadTest;
 import androidx.test.core.app.ApplicationProvider;
 
+import com.android.settings.network.telephony.scan.NetworkScanRepository;
+import com.android.settings.network.telephony.scan.NetworkScanRepository.NetworkScanResult;
 import com.android.settingslib.core.instrumentation.MetricsFeatureProvider;
 
+import com.google.common.collect.ImmutableList;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
@@ -163,8 +167,7 @@ public class NetworkSelectSettingsTest {
         }
 
         @Override
-        protected NetworkOperatorPreference
+        protected NetworkOperatorPreference createNetworkOperatorPreference(CellInfo cellInfo) {
-                createNetworkOperatorPreference(CellInfo cellInfo) {
             NetworkOperatorPreference pref = super.createNetworkOperatorPreference(cellInfo);
             if (cellInfo == mTestEnv.mCellInfo1) {
                 pref.updateCell(cellInfo, mTestEnv.mCellId1);
@@ -183,9 +186,14 @@ public class NetworkSelectSettingsTest {
     @Test
     @UiThreadTest
     public void updateAllPreferenceCategory_correctOrderingPreference() {
+        NetworkScanResult result = new NetworkScanResult(
+                NetworkScanRepository.NetworkScanState.COMPLETE,
+                ImmutableList.of(mCellInfo1, mCellInfo2));
         mNetworkSelectSettings.onCreateInitialization();
         mNetworkSelectSettings.enablePreferenceScreen(true);
-        mNetworkSelectSettings.scanResultHandler(Arrays.asList(mCellInfo1, mCellInfo2));
+
+        mNetworkSelectSettings.scanResultHandler(result);
+
         assertThat(mPreferenceCategory.getPreferenceCount()).isEqualTo(2);
         final NetworkOperatorPreference preference =
                 (NetworkOperatorPreference) mPreferenceCategory.getPreference(1);