diff options
5 files changed, 144 insertions, 68 deletions
diff --git a/service/java/com/android/server/wifi/AvailableNetworkNotifier.java b/service/java/com/android/server/wifi/AvailableNetworkNotifier.java index d738161cc..c28d52195 100644 --- a/service/java/com/android/server/wifi/AvailableNetworkNotifier.java +++ b/service/java/com/android/server/wifi/AvailableNetworkNotifier.java @@ -116,7 +116,7 @@ public class AvailableNetworkNotifier { private boolean mScreenOn; /** List of SSIDs blacklisted from recommendation. */ - private final Set<String> mBlacklistedSsids; + private final Set<String> mBlacklistedSsids = new ArraySet<>(); private final Context mContext; private final Handler mHandler; @@ -168,8 +168,6 @@ public class AvailableNetworkNotifier { mNotificationBuilder = connectToNetworkNotificationBuilder; mScreenOn = false; mSrcMessenger = new Messenger(new Handler(looper, mConnectionStateCallback)); - - mBlacklistedSsids = new ArraySet<>(); wifiConfigStore.registerStoreData(new SsidSetStoreData(mStoreDataIdentifier, new AvailableNetworkNotifierStoreData())); @@ -249,7 +247,7 @@ public class AvailableNetworkNotifier { Log.d(mTag, "Notification with state=" + mState + " was cleared for recommended network: " - + mRecommendedNetwork.SSID); + + "\"" + mRecommendedNetwork.SSID + "\""); } mState = STATE_NO_NOTIFICATION; mRecommendedNetwork = null; @@ -291,7 +289,7 @@ public class AvailableNetworkNotifier { if (mState == STATE_NO_NOTIFICATION || mState == STATE_SHOWING_RECOMMENDATION_NOTIFICATION) { ScanResult recommendation = - recommendNetwork(availableNetworks, new ArraySet<>(mBlacklistedSsids)); + recommendNetwork(availableNetworks); if (recommendation != null) { postInitialNotification(recommendation); @@ -304,9 +302,10 @@ public class AvailableNetworkNotifier { /** * Recommends a network to connect to from a list of available networks, while ignoring the * SSIDs in the blacklist. + * + * @param networks List of networks to select from */ - public ScanResult recommendNetwork(@NonNull List<ScanDetail> networks, - @NonNull Set<String> blacklistedSsids) { + public ScanResult recommendNetwork(@NonNull List<ScanDetail> networks) { ScanResult result = null; int highestRssi = Integer.MIN_VALUE; for (ScanDetail scanDetail : networks) { @@ -318,7 +317,7 @@ public class AvailableNetworkNotifier { } } - if (result != null && blacklistedSsids.contains(result.SSID)) { + if (result != null && mBlacklistedSsids.contains(result.SSID)) { result = null; } return result; @@ -333,8 +332,11 @@ public class AvailableNetworkNotifier { * Called by {@link WifiConnectivityManager} when Wi-Fi is connected. If the notification * was in the connecting state, update the notification to show that it has connected to the * recommended network. + * + * @param ssid The connected network's ssid */ - public void handleWifiConnected() { + public void handleWifiConnected(String ssid) { + removeNetworkFromBlacklist(ssid); if (mState != STATE_CONNECTING_IN_NOTIFICATION) { clearPendingNotification(true /* resetRepeatTime */); return; @@ -343,7 +345,8 @@ public class AvailableNetworkNotifier { postNotification(mNotificationBuilder.createNetworkConnectedNotification(mTag, mRecommendedNetwork)); - Log.d(mTag, "User connected to recommended network: " + mRecommendedNetwork.SSID); + Log.d(mTag, "User connected to recommended network: " + + "\"" + mRecommendedNetwork.SSID + "\""); mWifiMetrics.incrementConnectToNetworkNotification(mTag, ConnectToNetworkNotificationAndActionCount.NOTIFICATION_CONNECTED_TO_NETWORK); mState = STATE_CONNECTED_NOTIFICATION; @@ -365,7 +368,8 @@ public class AvailableNetworkNotifier { } postNotification(mNotificationBuilder.createNetworkFailedNotification(mTag)); - Log.d(mTag, "User failed to connect to recommended network: " + mRecommendedNetwork.SSID); + Log.d(mTag, "User failed to connect to recommended network: " + + "\"" + mRecommendedNetwork.SSID + "\""); mWifiMetrics.incrementConnectToNetworkNotification(mTag, ConnectToNetworkNotificationAndActionCount.NOTIFICATION_FAILED_TO_CONNECT); mState = STATE_CONNECT_FAILED_NOTIFICATION; @@ -418,7 +422,8 @@ public class AvailableNetworkNotifier { ConnectToNetworkNotificationAndActionCount.NOTIFICATION_CONNECTING_TO_NETWORK); Log.d(mTag, - "User initiated connection to recommended network: " + mRecommendedNetwork.SSID); + "User initiated connection to recommended network: " + + "\"" + mRecommendedNetwork.SSID + "\""); WifiConfiguration network = createRecommendedNetworkConfig(mRecommendedNetwork); Message msg = Message.obtain(); msg.what = WifiManager.CONNECT_NETWORK; @@ -443,7 +448,20 @@ public class AvailableNetworkNotifier { mWifiMetrics.setNetworkRecommenderBlacklistSize(mTag, mBlacklistedSsids.size()); mConfigManager.saveToStore(false /* forceWrite */); Log.d(mTag, "Network is added to the network notification blacklist: " - + ssid); + + "\"" + ssid + "\""); + } + + private void removeNetworkFromBlacklist(String ssid) { + if (ssid == null) { + return; + } + if (!mBlacklistedSsids.remove(ssid)) { + return; + } + mWifiMetrics.setNetworkRecommenderBlacklistSize(mTag, mBlacklistedSsids.size()); + mConfigManager.saveToStore(false /* forceWrite */); + Log.d(mTag, "Network is removed from the network notification blacklist: " + + "\"" + ssid + "\""); } WifiConfiguration createRecommendedNetworkConfig(ScanResult recommendedNetwork) { diff --git a/service/java/com/android/server/wifi/WifiConnectivityManager.java b/service/java/com/android/server/wifi/WifiConnectivityManager.java index 8b8229837..59dabdd63 100644 --- a/service/java/com/android/server/wifi/WifiConnectivityManager.java +++ b/service/java/com/android/server/wifi/WifiConnectivityManager.java @@ -1123,8 +1123,10 @@ public class WifiConnectivityManager { mWifiState = state; if (mWifiState == WIFI_STATE_CONNECTED) { - mOpenNetworkNotifier.handleWifiConnected(); - mCarrierNetworkNotifier.handleWifiConnected(); + mOpenNetworkNotifier.handleWifiConnected( + (mWifiInfo.getWifiSsid() == null) ? null : mWifiInfo.getWifiSsid().toString()); + mCarrierNetworkNotifier.handleWifiConnected( + (mWifiInfo.getWifiSsid() == null) ? null : mWifiInfo.getWifiSsid().toString()); } // Reset BSSID of last connection attempt and kick off diff --git a/tests/wifitests/src/com/android/server/wifi/CarrierNetworkNotifierTest.java b/tests/wifitests/src/com/android/server/wifi/CarrierNetworkNotifierTest.java index 734eefa64..9af9e7ae7 100644 --- a/tests/wifitests/src/com/android/server/wifi/CarrierNetworkNotifierTest.java +++ b/tests/wifitests/src/com/android/server/wifi/CarrierNetworkNotifierTest.java @@ -49,8 +49,6 @@ import android.os.UserManager; import android.os.test.TestLooper; import android.provider.Settings; import android.support.test.filters.SmallTest; -import android.util.ArraySet; - import com.android.server.wifi.nano.WifiMetricsProto.ConnectToNetworkNotificationAndActionCount; @@ -62,7 +60,6 @@ import org.mockito.MockitoAnnotations; import java.util.ArrayList; import java.util.List; -import java.util.Set; /** * Unit tests for {@link CarrierNetworkNotifier}. @@ -92,7 +89,6 @@ public class CarrierNetworkNotifierTest { private ContentObserver mContentObserver; private ScanResult mDummyNetwork; private List<ScanDetail> mCarrierNetworks; - private Set<String> mBlacklistedSsids; /** Initialize objects before each test run. */ @@ -115,7 +111,6 @@ public class CarrierNetworkNotifierTest { mDummyNetwork.level = MIN_RSSI_LEVEL; mCarrierNetworks = new ArrayList<>(); mCarrierNetworks.add(new ScanDetail(mDummyNetwork, null /* networkDetail */)); - mBlacklistedSsids = new ArraySet<>(); mLooper = new TestLooper(); mNotificationController = new CarrierNetworkNotifier( @@ -421,12 +416,10 @@ public class CarrierNetworkNotifierTest { mBroadcastReceiver.onReceive(mContext, createIntent(ACTION_CONNECT_TO_NETWORK)); verify(mWifiConfigManager).saveToStore(false /* forceWrite */); + verify(mWifiMetrics).setNetworkRecommenderBlacklistSize(CARRIER_NET_NOTIFIER_TAG, 1); - mNotificationController.clearPendingNotification(true); List<ScanDetail> scanResults = mCarrierNetworks; mNotificationController.handleScanResults(scanResults); - - verify(mWifiMetrics).setNetworkRecommenderBlacklistSize(CARRIER_NET_NOTIFIER_TAG, 1); } /** @@ -554,12 +547,12 @@ public class CarrierNetworkNotifierTest { } /** - * {@link CarrierNetworkNotifier#handleWifiConnected()} does not post connected notification if - * the connecting notification is not showing + * {@link CarrierNetworkNotifier#handleWifiConnected(String ssid)} does not post connected + * notification if the connecting notification is not showing */ @Test public void networkConnectionSuccess_wasNotInConnectingFlow_doesNothing() { - mNotificationController.handleWifiConnected(); + mNotificationController.handleWifiConnected(TEST_SSID_1); verify(mNotificationManager, never()).notify(anyInt(), any()); verify(mWifiMetrics, never()).incrementConnectToNetworkNotification( @@ -568,8 +561,8 @@ public class CarrierNetworkNotifierTest { } /** - * {@link CarrierNetworkNotifier#handleWifiConnected()} clears notification that is not - * connecting. + * {@link CarrierNetworkNotifier#handleWifiConnected(String ssid)} clears notification + * that is not connecting. */ @Test public void networkConnectionSuccess_wasShowingNotification_clearsNotification() { @@ -582,14 +575,14 @@ public class CarrierNetworkNotifierTest { ConnectToNetworkNotificationAndActionCount.NOTIFICATION_RECOMMEND_NETWORK); verify(mNotificationManager).notify(anyInt(), any()); - mNotificationController.handleWifiConnected(); + mNotificationController.handleWifiConnected(TEST_SSID_1); verify(mNotificationManager).cancel(anyInt()); } /** - * {@link CarrierNetworkNotifier#handleWifiConnected()} posts the connected notification if - * the connecting notification is showing. + * {@link CarrierNetworkNotifier#handleWifiConnected(String ssid)} posts the connected + * notification if the connecting notification is showing. */ @Test public void networkConnectionSuccess_wasInConnectingFlow_postsConnectedNotification() { @@ -614,7 +607,7 @@ public class CarrierNetworkNotifierTest { ConnectToNetworkNotificationAndActionCount.ACTION_CONNECT_TO_NETWORK); verify(mNotificationManager, times(2)).notify(anyInt(), any()); - mNotificationController.handleWifiConnected(); + mNotificationController.handleWifiConnected(TEST_SSID_1); // Connected Notification verify(mNotificationBuilder).createNetworkConnectedNotification(CARRIER_NET_NOTIFIER_TAG, @@ -748,8 +741,7 @@ public class CarrierNetworkNotifierTest { List<ScanDetail> scanResults = createCarrierScanResults(TEST_SSID_1); scanResults.get(0).getScanResult().level = MIN_RSSI_LEVEL; - ScanResult actual = mNotificationController.recommendNetwork( - scanResults, mBlacklistedSsids); + ScanResult actual = mNotificationController.recommendNetwork(scanResults); ScanResult expected = scanResults.get(0).getScanResult(); assertEquals(expected, actual); } @@ -761,8 +753,7 @@ public class CarrierNetworkNotifierTest { scanResults.get(0).getScanResult().level = MIN_RSSI_LEVEL; scanResults.get(1).getScanResult().level = MIN_RSSI_LEVEL + 1; - ScanResult actual = mNotificationController.recommendNetwork( - scanResults, mBlacklistedSsids); + ScanResult actual = mNotificationController.recommendNetwork(scanResults); ScanResult expected = scanResults.get(1).getScanResult(); assertEquals(expected, actual); } @@ -772,13 +763,49 @@ public class CarrierNetworkNotifierTest { */ @Test public void blacklistBestNetworkSsid_shouldNeverRecommendNetwork() { - List<ScanDetail> scanResults = createCarrierScanResults(TEST_SSID_1, TEST_SSID_2); + // Add TEST_SSID_1 to blacklist + userDismissedNotification_shouldBlacklistNetwork(); + + List<ScanDetail> scanResults = createCarrierScanResults(mDummyNetwork.SSID, TEST_SSID_2); scanResults.get(0).getScanResult().level = MIN_RSSI_LEVEL + 1; scanResults.get(1).getScanResult().level = MIN_RSSI_LEVEL; - mBlacklistedSsids.add(TEST_SSID_1); - ScanResult actual = mNotificationController.recommendNetwork( - scanResults, mBlacklistedSsids); + ScanResult actual = mNotificationController.recommendNetwork(scanResults); assertNull(actual); } + + /** + * Test null input is handled + */ + @Test + public void removeNetworkFromBlacklist_handlesNull() { + mNotificationController.handleWifiConnected(null); + verify(mWifiConfigManager, never()).saveToStore(false /* forceWrite */); + } + + /** + * If the blacklist didn't change then there is no need to continue further. + */ + @Test + public void removeNetworkFromBlacklist_returnsEarlyIfNothingIsRemoved() { + mNotificationController.handleWifiConnected(TEST_SSID_1); + verify(mWifiConfigManager, never()).saveToStore(false /* forceWrite */); + } + + /** + * If we connected to a blacklisted network, then remove it from the blacklist. + */ + @Test + public void connectToNetwork_shouldRemoveSsidFromBlacklist() { + // Add TEST_SSID_1 to blacklist + userDismissedNotification_shouldBlacklistNetwork(); + + // Simulate the user connecting to TEST_SSID_1 and verify it is removed from the blacklist + mNotificationController.handleWifiConnected(mDummyNetwork.SSID); + verify(mWifiConfigManager, times(2)).saveToStore(false /* forceWrite */); + verify(mWifiMetrics).setNetworkRecommenderBlacklistSize(CARRIER_NET_NOTIFIER_TAG, 0); + ScanResult actual = mNotificationController.recommendNetwork(mCarrierNetworks); + ScanResult expected = mCarrierNetworks.get(0).getScanResult(); + assertEquals(expected, actual); + } } diff --git a/tests/wifitests/src/com/android/server/wifi/OpenNetworkNotifierTest.java b/tests/wifitests/src/com/android/server/wifi/OpenNetworkNotifierTest.java index e86860bda..68af82461 100644 --- a/tests/wifitests/src/com/android/server/wifi/OpenNetworkNotifierTest.java +++ b/tests/wifitests/src/com/android/server/wifi/OpenNetworkNotifierTest.java @@ -49,7 +49,6 @@ import android.os.UserManager; import android.os.test.TestLooper; import android.provider.Settings; import android.support.test.filters.SmallTest; -import android.util.ArraySet; import com.android.server.wifi.nano.WifiMetricsProto.ConnectToNetworkNotificationAndActionCount; @@ -61,7 +60,6 @@ import org.mockito.MockitoAnnotations; import java.util.ArrayList; import java.util.List; -import java.util.Set; /** * Unit tests for {@link OpenNetworkNotifier}. @@ -91,8 +89,6 @@ public class OpenNetworkNotifierTest { private ContentObserver mContentObserver; private ScanResult mDummyNetwork; private List<ScanDetail> mOpenNetworks; - private Set<String> mBlacklistedSsids; - /** Initialize objects before each test run. */ @Before @@ -114,7 +110,6 @@ public class OpenNetworkNotifierTest { mDummyNetwork.level = MIN_RSSI_LEVEL; mOpenNetworks = new ArrayList<>(); mOpenNetworks.add(new ScanDetail(mDummyNetwork, null /* networkDetail */)); - mBlacklistedSsids = new ArraySet<>(); mLooper = new TestLooper(); mNotificationController = new OpenNetworkNotifier( @@ -418,12 +413,10 @@ public class OpenNetworkNotifierTest { mBroadcastReceiver.onReceive(mContext, createIntent(ACTION_CONNECT_TO_NETWORK)); verify(mWifiConfigManager).saveToStore(false /* forceWrite */); + verify(mWifiMetrics).setNetworkRecommenderBlacklistSize(OPEN_NET_NOTIFIER_TAG, 1); - mNotificationController.clearPendingNotification(true); List<ScanDetail> scanResults = mOpenNetworks; mNotificationController.handleScanResults(scanResults); - - verify(mWifiMetrics).setNetworkRecommenderBlacklistSize(OPEN_NET_NOTIFIER_TAG, 1); } /** @@ -550,12 +543,12 @@ public class OpenNetworkNotifierTest { } /** - * {@link OpenNetworkNotifier#handleWifiConnected()} does not post connected notification if - * the connecting notification is not showing + * {@link OpenNetworkNotifier#handleWifiConnected(String ssid)} does not post connected + * notification if the connecting notification is not showing */ @Test public void networkConnectionSuccess_wasNotInConnectingFlow_doesNothing() { - mNotificationController.handleWifiConnected(); + mNotificationController.handleWifiConnected(TEST_SSID_1); verify(mNotificationManager, never()).notify(anyInt(), any()); verify(mWifiMetrics, never()).incrementConnectToNetworkNotification( @@ -564,7 +557,8 @@ public class OpenNetworkNotifierTest { } /** - * {@link OpenNetworkNotifier#handleWifiConnected()} clears notification that is not connecting. + * {@link OpenNetworkNotifier#handleWifiConnected(String ssid)} clears notification that + * is not connecting. */ @Test public void networkConnectionSuccess_wasShowingNotification_clearsNotification() { @@ -577,14 +571,14 @@ public class OpenNetworkNotifierTest { ConnectToNetworkNotificationAndActionCount.NOTIFICATION_RECOMMEND_NETWORK); verify(mNotificationManager).notify(anyInt(), any()); - mNotificationController.handleWifiConnected(); + mNotificationController.handleWifiConnected(TEST_SSID_1); verify(mNotificationManager).cancel(anyInt()); } /** - * {@link OpenNetworkNotifier#handleWifiConnected()} posts the connected notification if - * the connecting notification is showing. + * {@link OpenNetworkNotifier#handleWifiConnected(String ssid)} posts the connected + * notification if the connecting notification is showing. */ @Test public void networkConnectionSuccess_wasInConnectingFlow_postsConnectedNotification() { @@ -609,7 +603,7 @@ public class OpenNetworkNotifierTest { ConnectToNetworkNotificationAndActionCount.ACTION_CONNECT_TO_NETWORK); verify(mNotificationManager, times(2)).notify(anyInt(), any()); - mNotificationController.handleWifiConnected(); + mNotificationController.handleWifiConnected(TEST_SSID_1); // Connected Notification verify(mNotificationBuilder).createNetworkConnectedNotification(OPEN_NET_NOTIFIER_TAG, @@ -741,8 +735,7 @@ public class OpenNetworkNotifierTest { List<ScanDetail> scanResults = createOpenScanResults(TEST_SSID_1); scanResults.get(0).getScanResult().level = MIN_RSSI_LEVEL; - ScanResult actual = mNotificationController.recommendNetwork( - scanResults, mBlacklistedSsids); + ScanResult actual = mNotificationController.recommendNetwork(scanResults); ScanResult expected = scanResults.get(0).getScanResult(); assertEquals(expected, actual); } @@ -754,8 +747,7 @@ public class OpenNetworkNotifierTest { scanResults.get(0).getScanResult().level = MIN_RSSI_LEVEL; scanResults.get(1).getScanResult().level = MIN_RSSI_LEVEL + 1; - ScanResult actual = mNotificationController.recommendNetwork( - scanResults, mBlacklistedSsids); + ScanResult actual = mNotificationController.recommendNetwork(scanResults); ScanResult expected = scanResults.get(1).getScanResult(); assertEquals(expected, actual); } @@ -765,13 +757,50 @@ public class OpenNetworkNotifierTest { */ @Test public void blacklistBestNetworkSsid_shouldNeverRecommendNetwork() { - List<ScanDetail> scanResults = createOpenScanResults(TEST_SSID_1, TEST_SSID_2); + // Add TEST_SSID_1 to blacklist + userDismissedNotification_shouldBlacklistNetwork(); + + // Scan result with blacklisted SSID + List<ScanDetail> scanResults = createOpenScanResults(mDummyNetwork.SSID, TEST_SSID_2); scanResults.get(0).getScanResult().level = MIN_RSSI_LEVEL + 1; scanResults.get(1).getScanResult().level = MIN_RSSI_LEVEL; - mBlacklistedSsids.add(TEST_SSID_1); - ScanResult actual = mNotificationController.recommendNetwork( - scanResults, mBlacklistedSsids); + ScanResult actual = mNotificationController.recommendNetwork(scanResults); assertNull(actual); } + + /** + * Test null input is handled + */ + @Test + public void removeNetworkFromBlacklist_handlesNull() { + mNotificationController.handleWifiConnected(null); + verify(mWifiConfigManager, never()).saveToStore(false /* forceWrite */); + } + + /** + * If the blacklist didn't change then there is no need to continue further. + */ + @Test + public void removeNetworkFromBlacklist_returnsEarlyIfNothingIsRemoved() { + mNotificationController.handleWifiConnected(TEST_SSID_1); + verify(mWifiConfigManager, never()).saveToStore(false /* forceWrite */); + } + + /** + * If we connected to a blacklisted network, then remove it from the blacklist. + */ + @Test + public void connectToNetwork_shouldRemoveSsidFromBlacklist() { + // Add TEST_SSID_1 to blacklist + userDismissedNotification_shouldBlacklistNetwork(); + + // Simulate the user connecting to TEST_SSID_1 and verify it is removed from the blacklist + mNotificationController.handleWifiConnected(mDummyNetwork.SSID); + verify(mWifiConfigManager, times(2)).saveToStore(false /* forceWrite */); + verify(mWifiMetrics).setNetworkRecommenderBlacklistSize(OPEN_NET_NOTIFIER_TAG, 0); + ScanResult actual = mNotificationController.recommendNetwork(mOpenNetworks); + ScanResult expected = mOpenNetworks.get(0).getScanResult(); + assertEquals(expected, actual); + } } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java index 7b3e4d9ec..625ed7180 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java @@ -668,10 +668,10 @@ public class WifiConnectivityManagerTest { @Test public void wifiConnected_openNetworkNotifierHandlesConnection() { // Set WiFi to connected state + mWifiInfo.setSSID(WifiSsid.createFromAsciiEncoded(CANDIDATE_SSID)); mWifiConnectivityManager.handleConnectionStateChanged( WifiConnectivityManager.WIFI_STATE_CONNECTED); - - verify(mOpenNetworkNotifier).handleWifiConnected(); + verify(mOpenNetworkNotifier).handleWifiConnected(CANDIDATE_SSID); } /** @@ -811,10 +811,10 @@ public class WifiConnectivityManagerTest { @Test public void wifiConnected_carrierNetworkNotifierHandlesConnection() { // Set WiFi to connected state + mWifiInfo.setSSID(WifiSsid.createFromAsciiEncoded(CANDIDATE_SSID)); mWifiConnectivityManager.handleConnectionStateChanged( WifiConnectivityManager.WIFI_STATE_CONNECTED); - - verify(mCarrierNetworkNotifier).handleWifiConnected(); + verify(mCarrierNetworkNotifier).handleWifiConnected(CANDIDATE_SSID); } /** |