From 65c8d1df691824347c952461b78b1f6227511aff Mon Sep 17 00:00:00 2001 From: cosmohsieh Date: Sat, 15 Dec 2018 01:20:27 +0800 Subject: [PATCH] Add internal WifiTracker to get correct AccessPoint for WifiNetworkRequestDialog Use WifiTracker to get correct and constantly updated Wifi status of AccessPoint. Bug: 117399926 Test: RunSettingsRoboTests Change-Id: I5e4316f6acb7787dcaab150a293068852beb76e0 --- .../wifi/NetworkRequestDialogFragment.java | 173 ++++++++++++++---- .../NetworkRequestDialogActivityTest.java | 8 + .../NetworkRequestDialogFragmentTest.java | 133 ++++++++------ 3 files changed, 213 insertions(+), 101 deletions(-) diff --git a/src/com/android/settings/wifi/NetworkRequestDialogFragment.java b/src/com/android/settings/wifi/NetworkRequestDialogFragment.java index c627a2e6628..30c2cd96f16 100644 --- a/src/com/android/settings/wifi/NetworkRequestDialogFragment.java +++ b/src/com/android/settings/wifi/NetworkRequestDialogFragment.java @@ -30,6 +30,7 @@ import android.os.Message; import android.widget.BaseAdapter; import androidx.annotation.NonNull; +import androidx.annotation.VisibleForTesting; import androidx.appcompat.app.AlertDialog; import androidx.preference.internal.PreferenceImageView; @@ -46,7 +47,10 @@ import com.android.settings.R; import com.android.settings.core.instrumentation.InstrumentedDialogFragment; import com.android.settings.wifi.NetworkRequestErrorDialogFragment.ERROR_DIALOG_TYPE; import com.android.settingslib.Utils; +import com.android.settingslib.core.lifecycle.Lifecycle; import com.android.settingslib.wifi.AccessPoint; +import com.android.settingslib.wifi.WifiTracker; +import com.android.settingslib.wifi.WifiTrackerFactory; import java.util.ArrayList; import java.util.List; @@ -62,10 +66,14 @@ public class NetworkRequestDialogFragment extends InstrumentedDialogFragment imp /** Message sent to us to stop scanning wifi and pop up timeout dialog. */ private static final int MESSAGE_STOP_SCAN_WIFI_LIST = 0; + /** Spec defines there should be 5 wifi ap on the list at most. */ + private static final int MAX_NUMBER_LIST_ITEM = 5; + /** Delayed time to stop scanning wifi. */ private static final int DELAY_TIME_STOP_SCAN_MS = 30 * 1000; private List mAccessPointList; + private FilterWifiTracker mFilterWifiTracker; private AccessPointAdapter mDialogAdapter; private NetworkRequestUserSelectionCallback mUserSelectionCallback; @@ -159,6 +167,19 @@ public class NetworkRequestDialogFragment extends InstrumentedDialogFragment imp if (wifiManager != null) { wifiManager.unregisterNetworkRequestMatchCallback(this); } + + if (mFilterWifiTracker != null) { + mFilterWifiTracker.onPause(); + } + } + + @Override + public void onDestroy() { + super.onDestroy(); + if (mFilterWifiTracker != null) { + mFilterWifiTracker.onDestroy(); + mFilterWifiTracker = null; + } } @Override @@ -172,6 +193,11 @@ public class NetworkRequestDialogFragment extends InstrumentedDialogFragment imp } // Sets time-out to stop scanning. mHandler.sendEmptyMessageDelayed(MESSAGE_STOP_SCAN_WIFI_LIST, DELAY_TIME_STOP_SCAN_MS); + + if (mFilterWifiTracker == null) { + mFilterWifiTracker = new FilterWifiTracker(getActivity(), getSettingsLifecycle()); + } + mFilterWifiTracker.onResume(); } private final Handler mHandler = new Handler() { @@ -268,17 +294,33 @@ public class NetworkRequestDialogFragment extends InstrumentedDialogFragment imp @Override public void onMatch(List scanResults) { - // TODO(b/119846365): Checks if we could escalate the converting effort. - // Converts ScanResult to WifiConfiguration. - List wifiConfigurations = null; - final WifiManager wifiManager = getContext().getApplicationContext() - .getSystemService(WifiManager.class); - if (wifiManager != null) { - wifiConfigurations = wifiManager.getAllMatchingWifiConfigs(scanResults); + mHandler.removeMessages(MESSAGE_STOP_SCAN_WIFI_LIST); + renewAccessPointList(scanResults); + + notifyAdapterRefresh(); + } + + // Updates internal AccessPoint list from WifiTracker. scanResults are used to update key list + // of AccessPoint, and could be null if there is no necessary to update key list. + private void renewAccessPointList(List scanResults) { + if (mFilterWifiTracker == null) { + return; } - setUpAccessPointList(wifiConfigurations); + // TODO(b/119846365): Checks if we could escalate the converting effort. + // Updates keys of scanResults into FilterWifiTracker for updating matched AccessPoints. + if (scanResults != null) { + mFilterWifiTracker.updateKeys(scanResults); + } + // Re-gets matched AccessPoints from WifiTracker. + final List list = getAccessPointList(); + list.clear(); + list.addAll(mFilterWifiTracker.getAccessPoints()); + } + + @VisibleForTesting + void notifyAdapterRefresh() { if (getDialogAdapter() != null) { getDialogAdapter().notifyDataSetChanged(); } @@ -286,48 +328,99 @@ public class NetworkRequestDialogFragment extends InstrumentedDialogFragment imp @Override public void onUserSelectionConnectSuccess(WifiConfiguration wificonfiguration) { - if (getDialogAdapter() != null) { - updateAccessPointListItem(wificonfiguration); - getDialogAdapter().notifyDataSetChanged(); - } + // Dismisses current dialog, since connection is success. + dismiss(); } @Override public void onUserSelectionConnectFailure(WifiConfiguration wificonfiguration) { - if (mDialogAdapter != null) { - updateAccessPointListItem(wificonfiguration); - getDialogAdapter().notifyDataSetChanged(); - } + stopScanningAndPopErrorDialog(ERROR_DIALOG_TYPE.ABORT); } - private void updateAccessPointListItem(WifiConfiguration wificonfiguration) { - if (wificonfiguration == null) { - return; + private final class FilterWifiTracker { + private final List mAccessPointKeys; + private final WifiTracker mWifiTracker; + + public FilterWifiTracker(Context context, Lifecycle lifecycle) { + mWifiTracker = WifiTrackerFactory.create(context, mWifiListener, + lifecycle, /* includeSaved */ true, /* includeScans */ true); + mAccessPointKeys = new ArrayList<>(); } - final List accessPointList = getAccessPointList(); - final int accessPointListSize = accessPointList.size(); + /** + * Updates key list from input. {@code onMatch()} may be called in multi-times according + * wifi scanning result, so needs patchwork here. + */ + public void updateKeys(List scanResults) { + for (ScanResult scanResult : scanResults) { + final String key = AccessPoint.getKey(scanResult); + if (!mAccessPointKeys.contains(key)) { + mAccessPointKeys.add(key); + } + } + } - for (int i = 0; i < accessPointListSize; i++) { - AccessPoint accessPoint = accessPointList.get(i); - // It is the same AccessPoint SSID, and should be replaced to update latest properties. - if (accessPoint.matches(wificonfiguration)) { - accessPointList.set(i, new AccessPoint(getContext(), wificonfiguration)); - break; + /** + * Returns only AccessPoints whose key is in {@code mAccessPointKeys}. + * + * @return List of matched AccessPoints. + */ + public List getAccessPoints() { + final List allAccessPoints = mWifiTracker.getAccessPoints(); + final List result = new ArrayList<>(); + + // The order should be kept, because order means wifi score (sorting in WifiTracker). + int count = 0; + for (AccessPoint accessPoint : allAccessPoints) { + final String key = accessPoint.getKey(); + if (mAccessPointKeys.contains(key)) { + result.add(accessPoint); + + count++; + // Limits how many count of items could show. + if (count >= MAX_NUMBER_LIST_ITEM) { + break; + } + } + } + + return result; + } + + private WifiTracker.WifiListener mWifiListener = new WifiTracker.WifiListener() { + + @Override + public void onWifiStateChanged(int state) { + notifyAdapterRefresh(); + } + + @Override + public void onConnectedChanged() { + notifyAdapterRefresh(); + } + + @Override + public void onAccessPointsChanged() { + notifyAdapterRefresh(); + } + }; + + public void onDestroy() { + if (mWifiTracker != null) { + mWifiTracker.onDestroy(); + } + } + + public void onResume() { + if (mWifiTracker != null) { + mWifiTracker.onStart(); + } + } + + public void onPause() { + if (mWifiTracker != null) { + mWifiTracker.onStop(); } } } - - private void setUpAccessPointList(List wifiConfigurations) { - // Grants for zero size input, since maybe current wifi is off or somethings are wrong. - if (wifiConfigurations == null) { - return; - } - - final List accessPointList = getAccessPointList(); - accessPointList.clear(); - for (WifiConfiguration config : wifiConfigurations) { - accessPointList.add(new AccessPoint(getContext(), config)); - } - } } diff --git a/tests/robotests/src/com/android/settings/wifi/NetworkRequestDialogActivityTest.java b/tests/robotests/src/com/android/settings/wifi/NetworkRequestDialogActivityTest.java index 48f8ec0099d..107da79d8c5 100644 --- a/tests/robotests/src/com/android/settings/wifi/NetworkRequestDialogActivityTest.java +++ b/tests/robotests/src/com/android/settings/wifi/NetworkRequestDialogActivityTest.java @@ -18,9 +18,13 @@ package com.android.settings.wifi; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.mock; + import androidx.appcompat.app.AlertDialog; import com.android.settings.testutils.shadow.ShadowAlertDialogCompat; +import com.android.settingslib.wifi.WifiTracker; +import com.android.settingslib.wifi.WifiTrackerFactory; import org.junit.Test; import org.junit.runner.RunWith; @@ -34,6 +38,10 @@ public class NetworkRequestDialogActivityTest { @Test public void LaunchActivity_shouldShowNetworkRequestDialog() { + // Mocks fake WifiTracker, in case of exception in NetworkRequestDialogFragment.onResume(). + WifiTracker wifiTracker = mock(WifiTracker.class); + WifiTrackerFactory.setTestingWifiTracker(wifiTracker); + Robolectric.setupActivity(NetworkRequestDialogActivity.class); AlertDialog alertDialog = ShadowAlertDialogCompat.getLatestAlertDialog(); diff --git a/tests/robotests/src/com/android/settings/wifi/NetworkRequestDialogFragmentTest.java b/tests/robotests/src/com/android/settings/wifi/NetworkRequestDialogFragmentTest.java index 343d170e7ed..e64fae714e6 100644 --- a/tests/robotests/src/com/android/settings/wifi/NetworkRequestDialogFragmentTest.java +++ b/tests/robotests/src/com/android/settings/wifi/NetworkRequestDialogFragmentTest.java @@ -51,6 +51,10 @@ import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; + +import com.android.settingslib.wifi.WifiTracker; +import com.android.settingslib.wifi.WifiTrackerFactory; + import org.robolectric.shadows.ShadowLooper; @RunWith(RobolectricTestRunner.class) @@ -58,16 +62,21 @@ import org.robolectric.shadows.ShadowLooper; public class NetworkRequestDialogFragmentTest { private static final String KEY_SSID = "key_ssid"; + private static final String KEY_SECURITY = "key_security"; private FragmentActivity mActivity; private NetworkRequestDialogFragment networkRequestDialogFragment; private Context mContext; + private WifiTracker mWifiTracker; @Before public void setUp() { mActivity = Robolectric.setupActivity(FragmentActivity.class); networkRequestDialogFragment = spy(NetworkRequestDialogFragment.newInstance()); mContext = spy(RuntimeEnvironment.application); + + mWifiTracker = mock(WifiTracker.class); + WifiTrackerFactory.setTestingWifiTracker(mWifiTracker); } @Test @@ -140,71 +149,47 @@ public class NetworkRequestDialogFragmentTest { } @Test - public void updateAccessPointList_onUserSelectionConnectSuccess_updateCorrectly() { - List accessPointList = spy(new ArrayList<>()); - Bundle bundle = new Bundle(); - bundle.putString(KEY_SSID, "Test AP 1"); - accessPointList.add(new AccessPoint(mContext, bundle)); - bundle.putString(KEY_SSID, "Test AP 2"); - accessPointList.add(new AccessPoint(mContext, bundle)); - bundle.putString(KEY_SSID, "Test AP 3"); - accessPointList.add(new AccessPoint(mContext, bundle)); - bundle.putString(KEY_SSID, "Test AP 4"); - accessPointList.add(new AccessPoint(mContext, bundle)); - + public void updateAccessPointList_onUserSelectionConnectSuccess_shouldCloseTheDialog() { + List accessPointList = createAccessPointList(); when(networkRequestDialogFragment.getAccessPointList()).thenReturn(accessPointList); networkRequestDialogFragment.show(mActivity.getSupportFragmentManager(), null); + AlertDialog alertDialog = ShadowAlertDialogCompat.getLatestAlertDialog(); + assertThat(alertDialog.isShowing()).isTrue(); // Test if config would update list. WifiConfiguration config = new WifiConfiguration(); config.SSID = "Test AP 3"; networkRequestDialogFragment.onUserSelectionConnectSuccess(config); - AccessPoint verifyAccessPoint = new AccessPoint(mContext, config); - verify(accessPointList, times(1)).set(2, verifyAccessPoint); + assertThat(alertDialog.isShowing()).isFalse(); } @Test - public void updateAccessPointList_onUserSelectionConnectFailure_updateCorrectly() { - List accessPointList = spy(new ArrayList<>()); - Bundle bundle = new Bundle(); - bundle.putString(KEY_SSID, "Test AP 1"); - accessPointList.add(new AccessPoint(mContext, bundle)); - bundle.putString(KEY_SSID, "Test AP 2"); - accessPointList.add(new AccessPoint(mContext, bundle)); - bundle.putString(KEY_SSID, "Test AP 3"); - accessPointList.add(new AccessPoint(mContext, bundle)); - bundle.putString(KEY_SSID, "Test AP 4"); - accessPointList.add(new AccessPoint(mContext, bundle)); + public void updateAccessPointList_onUserSelectionConnectFailure_shouldCallTimeoutDialog() { + FakeNetworkRequestDialogFragment fakeFragment = new FakeNetworkRequestDialogFragment(); + FakeNetworkRequestDialogFragment spyFakeFragment = spy(fakeFragment); + List accessPointList = createAccessPointList(); + when(spyFakeFragment.getAccessPointList()).thenReturn(accessPointList); + spyFakeFragment.show(mActivity.getSupportFragmentManager(), null); - when(networkRequestDialogFragment.getAccessPointList()).thenReturn(accessPointList); - networkRequestDialogFragment.show(mActivity.getSupportFragmentManager(), null); + AlertDialog alertDialog = ShadowAlertDialogCompat.getLatestAlertDialog(); + assertThat(alertDialog.isShowing()).isTrue(); // Test if config would update list. WifiConfiguration config = new WifiConfiguration(); config.SSID = "Test AP 3"; - networkRequestDialogFragment.onUserSelectionConnectFailure(config); + fakeFragment.onUserSelectionConnectFailure(config); - AccessPoint verifyAccessPoint = new AccessPoint(mContext, config); - verify(accessPointList, times(1)).set(2, verifyAccessPoint); + assertThat(fakeFragment.bCalledStopAndPop).isTrue(); } @Test - public void onUserSelectionCallbackRegistration_shouldCallSelect() { - List accessPointList = spy(new ArrayList<>()); - Bundle bundle = new Bundle(); - bundle.putString(KEY_SSID, "Test AP 1"); - accessPointList.add(new AccessPoint(mContext, bundle)); - bundle.putString(KEY_SSID, "Test AP 2"); - accessPointList.add(new AccessPoint(mContext, bundle)); - - bundle.putString(KEY_SSID, "Test AP 3"); - AccessPoint clickedAccessPoint = new AccessPoint(mContext, bundle); + public void onUserSelectionCallbackRegistration_onClick_shouldCallSelect() { + // Assert. + final int indexClickItem = 3; + List accessPointList = createAccessPointList(); + AccessPoint clickedAccessPoint = accessPointList.get(indexClickItem); clickedAccessPoint.generateOpenNetworkConfig(); - accessPointList.add(clickedAccessPoint); - - bundle.putString(KEY_SSID, "Test AP 4"); - accessPointList.add(new AccessPoint(mContext, bundle)); when(networkRequestDialogFragment.getAccessPointList()).thenReturn(accessPointList); NetworkRequestUserSelectionCallback selectionCallback = mock( @@ -212,40 +197,66 @@ public class NetworkRequestDialogFragmentTest { AlertDialog dialog = mock(AlertDialog.class); networkRequestDialogFragment.onUserSelectionCallbackRegistration(selectionCallback); - networkRequestDialogFragment.onClick(dialog, 2); + // Act. + networkRequestDialogFragment.onClick(dialog, indexClickItem); + // Check. verify(selectionCallback, times(1)).select(clickedAccessPoint.getConfig()); } @Test public void onMatch_shouldUpdatedList() { - // Prepares WifiManager. + // Assert. when(networkRequestDialogFragment.getContext()).thenReturn(mContext); Context applicationContext = spy(RuntimeEnvironment.application.getApplicationContext()); when(mContext.getApplicationContext()).thenReturn(applicationContext); WifiManager wifiManager = mock(WifiManager.class); when(applicationContext.getSystemService(Context.WIFI_SERVICE)).thenReturn(wifiManager); + networkRequestDialogFragment.onResume(); + + List accessPointList = createAccessPointList(); + when(mWifiTracker.getAccessPoints()).thenReturn(accessPointList); - List wifiConfigurationList = new ArrayList<>(); - WifiConfiguration config = new WifiConfiguration(); final String SSID_AP1 = "Test AP 1"; - config.SSID = SSID_AP1; - wifiConfigurationList.add(config); - config = new WifiConfiguration(); final String SSID_AP2 = "Test AP 2"; - config.SSID = SSID_AP2; - wifiConfigurationList.add(config); - - // Prepares callback converted data. List scanResults = new ArrayList<>(); - when(wifiManager.getAllMatchingWifiConfigs(scanResults)).thenReturn(wifiConfigurationList); + ScanResult scanResult = new ScanResult(); + scanResult.SSID = SSID_AP1; + scanResult.capabilities = "WEP"; + scanResults.add(scanResult); + scanResult = new ScanResult(); + scanResult.SSID = SSID_AP2; + scanResult.capabilities = "WEP"; + scanResults.add(scanResult); + // Act. networkRequestDialogFragment.onMatch(scanResults); - List accessPointList = networkRequestDialogFragment.getAccessPointList(); - assertThat(accessPointList).isNotEmpty(); - assertThat(accessPointList.size()).isEqualTo(2); - assertThat(accessPointList.get(0).getSsid()).isEqualTo(SSID_AP1); - assertThat(accessPointList.get(1).getSsid()).isEqualTo(SSID_AP2); + // Check. + List returnList = networkRequestDialogFragment.getAccessPointList(); + assertThat(returnList).isNotEmpty(); + assertThat(returnList.size()).isEqualTo(2); + assertThat(returnList.get(0).getSsid()).isEqualTo(SSID_AP1); + assertThat(returnList.get(1).getSsid()).isEqualTo(SSID_AP2); + } + + private List createAccessPointList() { + List accessPointList = spy(new ArrayList<>()); + Bundle bundle = new Bundle(); + bundle.putString(KEY_SSID, "Test AP 1"); + bundle.putInt(KEY_SECURITY, 1); + accessPointList.add(new AccessPoint(mContext, bundle)); + bundle.putString(KEY_SSID, "Test AP 2"); + bundle.putInt(KEY_SECURITY, 1); + accessPointList.add(new AccessPoint(mContext, bundle)); + bundle.putString(KEY_SSID, "Test AP 3"); + bundle.putInt(KEY_SECURITY, 2); + AccessPoint clickedAccessPoint = new AccessPoint(mContext, bundle); + accessPointList.add(clickedAccessPoint); + bundle.putString(KEY_SSID, "Test AP 4"); + bundle.putInt(KEY_SECURITY, 0); + accessPointList.add(new AccessPoint(mContext, bundle)); + + return accessPointList; } }