From 4205874ec745d8b254f283bbee12e2f41a57f8e6 Mon Sep 17 00:00:00 2001 From: Sundeep Ghuman Date: Fri, 21 Jul 2017 18:42:10 -0700 Subject: [PATCH] Do not evict scan results in cold start. When starting WifiTracker, show whatever results are available from platform until new scan results come in. This allows us to show the best available results from the previous location scan. Once we have resumed scanning and new scan results come in, the previous logic of evicting scan results older than 15 seconds applies. Also prevent unnecessary onConnectedChanged callbacks from being fired, which may unnecessarily cause UI reloading bars to appear by performing unnecessary work. Bug: b/38212080 Test: runtest --path frameworks/base/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/AccessPointTest.java && runtest --path frameworks/base/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java Change-Id: I3db9d98425b3d2fc66fa0757807a318d0c7456ee --- .../android/settingslib/wifi/AccessPoint.java | 5 +++ .../android/settingslib/wifi/WifiTracker.java | 29 +++++++------- .../settingslib/wifi/WifiTrackerTest.java | 39 +++++++++++++++++++ 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java b/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java index ec3b520e2012d..5015df77e4964 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java @@ -118,6 +118,7 @@ public class AccessPoint implements Comparable { */ private final ConcurrentHashMap mScanResultCache = new ConcurrentHashMap(32); + /** Maximum age of scan results to hold onto while actively scanning. **/ private static final long MAX_SCAN_RESULT_AGE_MS = 15000; static final String KEY_NETWORKINFO = "key_networkinfo"; @@ -441,6 +442,10 @@ public class AccessPoint implements Comparable { } private void evictOldScanResults() { + if (WifiTracker.sStaleScanResults) { + // Do not evict old scan results unless we are scanning and have fresh results. + return; + } long nowMs = SystemClock.elapsedRealtime(); for (Iterator iter = mScanResultCache.values().iterator(); iter.hasNext(); ) { ScanResult result = iter.next(); diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java index 9ccd33226258a..04bb802b46d2d 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java @@ -143,7 +143,9 @@ public class WifiTracker { @VisibleForTesting Scanner mScanner; - private boolean mStaleScanResults = true; + + @GuardedBy("mLock") + static boolean sStaleScanResults = true; public WifiTracker(Context context, WifiListener wifiListener, boolean includeSaved, boolean includeScans) { @@ -237,9 +239,7 @@ public class WifiTracker { }; } - /** - * Synchronously update the list of access points with the latest information. - */ + /** Synchronously update the list of access points with the latest information. */ @MainThread public void forceUpdate() { synchronized (mLock) { @@ -249,6 +249,7 @@ public class WifiTracker { final List newScanResults = mWifiManager.getScanResults(); List configs = mWifiManager.getConfiguredNetworks(); + mInternalAccessPoints.clear(); updateAccessPointsLocked(newScanResults, configs); if (DBG) { @@ -353,7 +354,7 @@ public class WifiTracker { *

This should always be called when done with a WifiTracker (if startTracking was called) to * ensure proper cleanup and prevent any further callbacks from occurring. * - *

Calling this method will set the {@link #mStaleScanResults} bit, which prevents + *

Calling this method will set the {@link #sStaleScanResults} bit, which prevents * {@link WifiListener#onAccessPointsChanged()} callbacks from being invoked (until the bit * is unset on the next SCAN_RESULTS_AVAILABLE_ACTION). */ @@ -371,8 +372,8 @@ public class WifiTracker { mWorkHandler.removePendingMessages(); mMainHandler.removePendingMessages(); + sStaleScanResults = true; } - mStaleScanResults = true; } private void unregisterAndClearScoreCache() { @@ -474,14 +475,14 @@ public class WifiTracker { /** * Safely modify {@link #mInternalAccessPoints} by acquiring {@link #mLock} first. * - *

Will not perform the update if {@link #mStaleScanResults} is true + *

Will not perform the update if {@link #sStaleScanResults} is true */ private void updateAccessPoints() { List configs = mWifiManager.getConfiguredNetworks(); final List newScanResults = mWifiManager.getScanResults(); synchronized (mLock) { - if(!mStaleScanResults) { + if(!sStaleScanResults) { updateAccessPointsLocked(newScanResults, configs); } } @@ -491,7 +492,7 @@ public class WifiTracker { * Update the internal list of access points. * *

Do not called directly (except for forceUpdate), use {@link #updateAccessPoints()} which - * respects {@link #mStaleScanResults}. + * respects {@link #sStaleScanResults}. */ @GuardedBy("mLock") private void updateAccessPointsLocked(final List newScanResults, @@ -781,9 +782,11 @@ public class WifiTracker { mWorkHandler.sendEmptyMessage(WorkHandler.MSG_UPDATE_ACCESS_POINTS); } else if (WifiManager.NETWORK_STATE_CHANGED_ACTION.equals(action)) { NetworkInfo info = intent.getParcelableExtra(WifiManager.EXTRA_NETWORK_INFO); - mConnected.set(info.isConnected()); - mMainHandler.sendEmptyMessage(MainHandler.MSG_CONNECTED_CHANGED); + if(mConnected.get() != info.isConnected()) { + mConnected.set(info.isConnected()); + mMainHandler.sendEmptyMessage(MainHandler.MSG_CONNECTED_CHANGED); + } mWorkHandler.obtainMessage(WorkHandler.MSG_UPDATE_NETWORK_INFO, info) .sendToTarget(); @@ -836,7 +839,7 @@ public class WifiTracker { // Only notify listeners of changes if we have fresh scan results, otherwise the // UI will be updated with stale results. We want to copy the APs regardless, // for instances where forceUpdate was invoked by the caller. - if (mStaleScanResults) { + if (sStaleScanResults) { copyAndNotifyListeners(false /*notifyListeners*/); } else { copyAndNotifyListeners(true /*notifyListeners*/); @@ -891,7 +894,7 @@ public class WifiTracker { switch (msg.what) { case MSG_UPDATE_ACCESS_POINTS: if (msg.arg1 == CLEAR_STALE_SCAN_RESULTS) { - mStaleScanResults = false; + sStaleScanResults = false; } updateAccessPoints(); break; diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java index b6d0c457db7c7..fdbf393d7e06e 100644 --- a/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java +++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java @@ -135,7 +135,9 @@ public class WifiTrackerTest { private HandlerThread mWorkerThread; private Looper mWorkerLooper; private Looper mMainLooper; + private int mOriginalScoringUiSettingValue; + private boolean mOriginalStaleScanResultsValue; @Before public void setUp() { @@ -210,6 +212,8 @@ public class WifiTrackerTest { InstrumentationRegistry.getTargetContext().getContentResolver(), Settings.Global.NETWORK_SCORING_UI_ENABLED, 1 /* enabled */); + + mOriginalStaleScanResultsValue = WifiTracker.sStaleScanResults; } @After @@ -218,6 +222,8 @@ public class WifiTrackerTest { InstrumentationRegistry.getTargetContext().getContentResolver(), Settings.Global.NETWORK_SCORING_UI_ENABLED, mOriginalScoringUiSettingValue); + + WifiTracker.sStaleScanResults = mOriginalStaleScanResultsValue; } private static ScanResult buildScanResult1() { @@ -829,4 +835,37 @@ public class WifiTrackerTest { assertThat(tracker.getAccessPoints()).isEmpty(); } + + @Test + public void onConnectedChangedCallback_shouldNotBeInvokedWhenNoStateChange() throws Exception { + WifiTracker tracker = createTrackerWithScanResultsAndAccessPoint1Connected(); + verify(mockWifiListener, times(1)).onConnectedChanged(); + + NetworkInfo networkInfo = new NetworkInfo( + ConnectivityManager.TYPE_WIFI, 0, "Type Wifi", "subtype"); + networkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTED, "connected", "test"); + + Intent intent = new Intent(WifiManager.NETWORK_STATE_CHANGED_ACTION); + intent.putExtra(WifiManager.EXTRA_NETWORK_INFO, networkInfo); + tracker.mReceiver.onReceive(mContext, intent); + + verify(mockWifiListener, times(1)).onConnectedChanged(); + } + + @Test + public void onConnectedChangedCallback_shouldNBeInvokedWhenStateChanges() throws Exception { + WifiTracker tracker = createTrackerWithScanResultsAndAccessPoint1Connected(); + verify(mockWifiListener, times(1)).onConnectedChanged(); + + NetworkInfo networkInfo = new NetworkInfo( + ConnectivityManager.TYPE_WIFI, 0, "Type Wifi", "subtype"); + networkInfo.setDetailedState( + NetworkInfo.DetailedState.DISCONNECTED, "dicconnected", "test"); + + Intent intent = new Intent(WifiManager.NETWORK_STATE_CHANGED_ACTION); + intent.putExtra(WifiManager.EXTRA_NETWORK_INFO, networkInfo); + tracker.mReceiver.onReceive(mContext, intent); + + verify(mockWifiListener, times(2)).onConnectedChanged(); + } }