Remove the double lists in WifiTracker. (bb39991c) · Commits · e / os / android_frameworks_base

packages/SettingsLib/src/com/android/settingslib/wifi/AccessPoint.java

+51 −40

Original line number	Diff line number	Diff line
		@@ -17,6 +17,7 @@
		package com.android.settingslib.wifi;

		import android.annotation.IntDef;
		import android.annotation.MainThread;
		import android.annotation.Nullable;
		import android.app.AppGlobals;
		import android.content.Context;
		@@ -42,6 +43,8 @@ import android.net.wifi.WifiManager;
		import android.net.wifi.WifiNetworkScoreCache;
		import android.net.wifi.hotspot2.PasspointConfiguration;
		import android.os.Bundle;
		import android.os.Handler;
		import android.os.Looper;
		import android.os.Parcelable;
		import android.os.RemoteException;
		import android.os.ServiceManager;
		@@ -57,6 +60,7 @@ import android.util.Log;

		import com.android.internal.annotations.VisibleForTesting;
		import com.android.settingslib.R;
		import com.android.settingslib.utils.ThreadUtils;

		import java.lang.annotation.Retention;
		import java.lang.annotation.RetentionPolicy;
		@@ -68,7 +72,14 @@ import java.util.Map;
		import java.util.Set;
		import java.util.concurrent.atomic.AtomicInteger;


		/**
		* Represents a selectable Wifi Network for use in various wifi selection menus backed by
		* {@link WifiTracker}.
		*
		* <p>An AccessPoint, which would be more fittingly named "WifiNetwork", is an aggregation of
		* {@link ScanResult ScanResults} along with pertinent metadata (e.g. current connection info,
		* network scores) required to successfully render the network to the user.
		*/
		public class AccessPoint implements Comparable<AccessPoint> {
		static final String TAG = "SettingsLib.AccessPoint";

		@@ -288,11 +299,6 @@ public class AccessPoint implements Comparable<AccessPoint> {
		mId = sLastId.incrementAndGet();
		}

		AccessPoint(Context context, AccessPoint other) {
		mContext = context;
		copyFrom(other);
		}

		AccessPoint(Context context, Collection<ScanResult> results) {
		mContext = context;

		@@ -345,33 +351,6 @@ public class AccessPoint implements Comparable<AccessPoint> {
		mKey = builder.toString();
		}

		/**
		* Copy accesspoint information. NOTE: We do not copy tag information because that is never
		* set on the internal copy.
		*/
		void copyFrom(AccessPoint that) {
		this.ssid = that.ssid;
		this.bssid = that.bssid;
		this.security = that.security;
		this.mKey = that.mKey;
		this.networkId = that.networkId;
		this.pskType = that.pskType;
		this.mConfig = that.mConfig; //TODO: Watch out, this object is mutated.
		this.mRssi = that.mRssi;
		this.mInfo = that.mInfo;
		this.mNetworkInfo = that.mNetworkInfo;
		this.mScanResults.clear();
		this.mScanResults.addAll(that.mScanResults);
		this.mScoredNetworkCache.clear();
		this.mScoredNetworkCache.putAll(that.mScoredNetworkCache);
		this.mId = that.mId;
		this.mSpeed = that.mSpeed;
		this.mIsScoredNetworkMetered = that.mIsScoredNetworkMetered;
		this.mIsCarrierAp = that.mIsCarrierAp;
		this.mCarrierApEapType = that.mCarrierApEapType;
		this.mCarrierName = that.mCarrierName;
		}

		/**
		* Returns a negative integer, zero, or a positive integer if this AccessPoint is less than,
		* equal to, or greater than the other AccessPoint.
		@@ -1070,12 +1049,12 @@ public class AccessPoint implements Comparable<AccessPoint> {
		// Only update labels on visible rssi changes
		updateSpeed();
		if (mAccessPointListener != null) {
		mAccessPointListener.onLevelChanged(this);
		ThreadUtils.postOnMainThread(() -> mAccessPointListener.onLevelChanged(this));
		}
		}

		if (mAccessPointListener != null) {
		mAccessPointListener.onAccessPointChanged(this);
		ThreadUtils.postOnMainThread(() -> mAccessPointListener.onAccessPointChanged(this));
		}

		if (!scanResults.isEmpty()) {
		@@ -1123,10 +1102,10 @@ public class AccessPoint implements Comparable<AccessPoint> {
		mNetworkInfo = null;
		}
		if (updated && mAccessPointListener != null) {
		mAccessPointListener.onAccessPointChanged(this);
		ThreadUtils.postOnMainThread(() -> mAccessPointListener.onAccessPointChanged(this));

		if (oldLevel != getLevel() /* current level */) {
		mAccessPointListener.onLevelChanged(this);
		ThreadUtils.postOnMainThread(() -> mAccessPointListener.onLevelChanged(this));
		}
		}

		@@ -1137,7 +1116,7 @@ public class AccessPoint implements Comparable<AccessPoint> {
		mConfig = config;
		networkId = config != null ? config.networkId : WifiConfiguration.INVALID_NETWORK_ID;
		if (mAccessPointListener != null) {
		mAccessPointListener.onAccessPointChanged(this);
		ThreadUtils.postOnMainThread(() -> mAccessPointListener.onAccessPointChanged(this));
		}
		}

		@@ -1333,9 +1312,41 @@ public class AccessPoint implements Comparable<AccessPoint> {
		return string;
		}

		/**
		* Callbacks relaying changes to the AccessPoint representation.
		*
		* <p>All methods are invoked on the Main Thread.
		*/
		public interface AccessPointListener {
		void onAccessPointChanged(AccessPoint accessPoint);
		void onLevelChanged(AccessPoint accessPoint);
		/**
		* Indicates a change to the externally visible state of the AccessPoint trigger by an
		* update of ScanResults, saved configuration state, connection state, or score
		* (labels/metered) state.
		*
		* <p>Clients should refresh their view of the AccessPoint to match the updated state when
		* this is invoked. Overall this method is extraneous if clients are listening to
		* {@link WifiTracker.WifiListener#onAccessPointsChanged()} callbacks.
		*
		* <p>Examples of changes include signal strength, connection state, speed label, and
		* generally anything that would impact the summary string.
		*
		* @param accessPoint The accessPoint object the listener was registered on which has
		* changed
		*/
		@MainThread void onAccessPointChanged(AccessPoint accessPoint);

		/**
		* Indicates the "wifi pie signal level" has changed, retrieved via calls to
		* {@link AccessPoint#getLevel()}.
		*
		* <p>This call is a subset of {@link #onAccessPointChanged(AccessPoint)} , hence is also
		* extraneous if the client is already reacting to that or the
		* {@link WifiTracker.WifiListener#onAccessPointsChanged()} callbacks.
		*
		* @param accessPoint The accessPoint object the listener was registered on whose level has
		* changed
		*/
		@MainThread void onLevelChanged(AccessPoint accessPoint);
		}

		private static boolean isVerboseLoggingEnabled() {

packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java

+44 −142

Original line number	Diff line number	Diff line
		@@ -15,6 +15,7 @@
		*/
		package com.android.settingslib.wifi;

		import android.annotation.AnyThread;
		import android.annotation.MainThread;
		import android.content.BroadcastReceiver;
		import android.content.Context;
		@@ -48,7 +49,6 @@ import android.text.format.DateUtils;
		import android.util.ArrayMap;
		import android.util.ArraySet;
		import android.util.Log;
		import android.util.SparseArray;
		import android.util.SparseIntArray;
		import android.widget.Toast;

		@@ -122,35 +122,17 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		@GuardedBy("mLock")
		private boolean mRegistered;

		/**
		* The externally visible access point list.
		*
		* Updated using main handler. Clone of this collection is returned from
		* {@link #getAccessPoints()}
		*/
		private final List<AccessPoint> mAccessPoints = new ArrayList<>();

		/**
		* The internal list of access points, synchronized on itself.
		*
		* Never exposed outside this class.
		*/
		/** The list of AccessPoints, aggregated visible ScanResults with metadata. */
		@GuardedBy("mLock")
		private final List<AccessPoint> mInternalAccessPoints = new ArrayList<>();

		/**
		* Synchronization lock for managing concurrency between main and worker threads.
		*
		* <p>This lock should be held for all background work.
		* TODO(b/37674366): Remove the worker thread so synchronization is no longer necessary.
		* <p>This lock should be held for all modifications to {@link #mInternalAccessPoints}.
		*/
		private final Object mLock = new Object();

		//visible to both worker and main thread.
		@GuardedBy("mLock")
		private final AccessPointListenerAdapter mAccessPointListenerAdapter
		= new AccessPointListenerAdapter();

		private final HashMap<String, Integer> mSeenBssids = new HashMap<>();

		// TODO(sghuman): Change this to be keyed on AccessPoint.getKey
		@@ -170,6 +152,12 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		@VisibleForTesting
		Scanner mScanner;

		/**
		* Tracks whether fresh scan results have been received since scanning start.
		*
		* <p>If this variable is false, we will not evict the scan result cache or invoke callbacks
		* so that we do not update the UI with stale data / clear out existing UI elements prematurely.
		*/
		@GuardedBy("mLock")
		private boolean mStaleScanResults = true;

		@@ -285,12 +273,6 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		List<WifiConfiguration> configs = mWifiManager.getConfiguredNetworks();
		mInternalAccessPoints.clear();
		updateAccessPointsLocked(newScanResults, configs);

		// Synchronously copy access points
		copyAndNotifyListeners();
		if (isVerboseLoggingEnabled()) {
		Log.i(TAG, "force update - external access point list:\n" + mAccessPoints);
		}
		}
		}

		@@ -418,12 +400,19 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		}

		/**
		* Gets the current list of access points. Should be called from main thread, otherwise
		* expect inconsistencies
		* Gets the current list of access points.
		*
		* <p>This method is can be called on an abitrary thread by clients, but is normally called on
		* the UI Thread by the rendering App.
		*/
		@MainThread
		@AnyThread
		public List<AccessPoint> getAccessPoints() {
		return new ArrayList<>(mAccessPoints);
		// TODO(sghuman): Investigate how to eliminate or reduce the need for locking now that we
		// have transitioned to a single worker thread model.

		synchronized (mLock) {
		return new ArrayList<>(mInternalAccessPoints);
		}
		}

		public WifiManager getManager() {
		@@ -535,11 +524,15 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		* Update the internal list of access points.
		*
		* <p>Do not call directly (except for forceUpdate), use {@link #updateAccessPoints()} which
		* respects {@link #mStaleScanResults}.
		* acquires the lock first.
		*/
		@GuardedBy("mLock")
		private void updateAccessPointsLocked(final List<ScanResult> newScanResults,
		List<WifiConfiguration> configs) {
		// TODO(sghuman): Reduce the synchronization time by only holding the lock when
		// modifying lists exposed to operations on the MainThread (getAccessPoints, stopTracking,
		// startTracking, etc).

		WifiConfiguration connectionConfig = null;
		if (mLastInfo != null) {
		connectionConfig = getWifiConfigurationForNetworkId(
		@@ -645,7 +638,7 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		mInternalAccessPoints.clear();
		mInternalAccessPoints.addAll(accessPoints);

		copyAndNotifyListeners();
		conditionallyNotifyListeners();
		}

		@VisibleForTesting
		@@ -661,7 +654,6 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		}
		}
		final AccessPoint accessPoint = new AccessPoint(mContext, scanResults);
		accessPoint.setListener(mAccessPointListenerAdapter);
		return accessPoint;
		}

		@@ -672,11 +664,11 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		if (cache.get(i).matches(config)) {
		AccessPoint ret = cache.remove(i);
		ret.loadConfig(config);

		return ret;
		}
		}
		final AccessPoint accessPoint = new AccessPoint(mContext, config);
		accessPoint.setListener(mAccessPointListenerAdapter);
		return accessPoint;
		}

		@@ -726,16 +718,21 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		Collections.sort(mInternalAccessPoints);
		}
		if (updated) {
		copyAndNotifyListeners();
		conditionallyNotifyListeners();
		}
		}
		}

		/**
		* Clears the access point list and conditionally invokes
		* {@link WifiListener#onAccessPointsChanged()} if required (i.e. the list was not already
		* empty).
		*/
		private void clearAccessPointsAndConditionallyUpdate() {
		synchronized (mLock) {
		if (!mInternalAccessPoints.isEmpty()) {
		mInternalAccessPoints.clear();
		copyAndNotifyListeners();
		mListener.onAccessPointsChanged();
		}
		}
		}
		@@ -758,7 +755,7 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		}
		if (updated) {
		Collections.sort(mInternalAccessPoints);
		copyAndNotifyListeners();
		conditionallyNotifyListeners();
		}
		}
		}
		@@ -1003,122 +1000,27 @@ public class WifiTracker implements LifecycleObserver, OnStart, OnStop, OnDestro
		void onWifiStateChanged(int state);

		/**
		* Called when the connection state of wifi has changed and isConnected
		* should be called to get the updated state.
		* Called when the connection state of wifi has changed and
		* {@link WifiTracker#isConnected()} should be called to get the updated state.
		*/
		void onConnectedChanged();

		/**
		* Called to indicate the list of AccessPoints has been updated and
		* getAccessPoints should be called to get the latest information.
		* {@link WifiTracker#getAccessPoints()} should be called to get the updated list.
		*/
		void onAccessPointsChanged();
		}

		/**
		* Helps capture notifications that were generated during AccessPoint modification. Used later
		* on by {@link #copyAndNotifyListeners()} to send notifications.
		*/
		private static class AccessPointListenerAdapter implements AccessPoint.AccessPointListener {
		static final int AP_CHANGED = 1;
		static final int LEVEL_CHANGED = 2;

		final SparseIntArray mPendingNotifications = new SparseIntArray();

		@Override
		public void onAccessPointChanged(AccessPoint accessPoint) {
		int type = mPendingNotifications.get(accessPoint.mId);
		mPendingNotifications.put(accessPoint.mId, type \| AP_CHANGED);
		}

		@Override
		public void onLevelChanged(AccessPoint accessPoint) {
		int type = mPendingNotifications.get(accessPoint.mId);
		mPendingNotifications.put(accessPoint.mId, type \| LEVEL_CHANGED);
		}
		}

		/**
		* Responsible for copying access points from {@link #mInternalAccessPoints} and notifying
		* accesspoint and wifi listeners.
		*
		* <p>If {@link #mStaleScanResults} is false, listeners are notified, otherwise notifications
		* are dropped.
		* Invokes {@link WifiListenerWrapper#onAccessPointsChanged()} if {@link #mStaleScanResults}
		* is false.
		*/
		private void copyAndNotifyListeners() {
		// Need to watch out for memory allocations on main thread.
		SparseArray<AccessPoint> oldAccessPoints = new SparseArray<>();
		SparseIntArray notificationMap = null;
		List<AccessPoint> updatedAccessPoints = new ArrayList<>();

		for (AccessPoint accessPoint : mAccessPoints) {
		oldAccessPoints.put(accessPoint.mId, accessPoint);
		}

		synchronized (mLock) {
		if (DBG()) {
		Log.d(TAG, "Starting to copy AP items. Internal APs: "
		+ mInternalAccessPoints);
		}

		if (!mStaleScanResults) {
		notificationMap = mAccessPointListenerAdapter.mPendingNotifications.clone();
		}

		mAccessPointListenerAdapter.mPendingNotifications.clear();

		for (AccessPoint internalAccessPoint : mInternalAccessPoints) {
		AccessPoint accessPoint = oldAccessPoints.get(internalAccessPoint.mId);
		if (accessPoint == null) {
		accessPoint = new AccessPoint(mContext, internalAccessPoint);
		} else {
		accessPoint.copyFrom(internalAccessPoint);
		}
		updatedAccessPoints.add(accessPoint);
		}
		}

		mAccessPoints.clear();
		mAccessPoints.addAll(updatedAccessPoints);

		if (DBG()) {
		Log.d(TAG, "External APs after copying: " + mAccessPoints);
		}

		if (notificationMap != null && notificationMap.size() > 0) {
		for (AccessPoint accessPoint : updatedAccessPoints) {
		int notificationType = notificationMap.get(accessPoint.mId);
		AccessPoint.AccessPointListener listener = accessPoint.mAccessPointListener;
		if (notificationType == 0 \|\| listener == null) {
		continue;
		}

		if ((notificationType & AccessPointListenerAdapter.AP_CHANGED) != 0) {
		if (isVerboseLoggingEnabled()) {
		Log.i(TAG, String.format(
		"Invoking AccessPointListenerAdapter AP_CHANGED for AP key %s and "
		+ "listener %s on the MainThread",
		accessPoint.getKey(),
		listener));
		}
		ThreadUtils.postOnMainThread(() -> listener.onAccessPointChanged(accessPoint));
		}

		if ((notificationType & AccessPointListenerAdapter.LEVEL_CHANGED) != 0) {
		if (isVerboseLoggingEnabled()) {
		Log.i(TAG, String.format(
		"Invoking AccessPointListenerAdapter LEVEL_CHANGED for AP key %s "
		+ "and listener %s on the MainThread",
		accessPoint.getKey(),
		listener));
		}
		ThreadUtils.postOnMainThread(() -> listener.onLevelChanged(accessPoint));
		}
		}
		private void conditionallyNotifyListeners() {
		if (mStaleScanResults) {
		return;
		}

		if (!mStaleScanResults) {
		mListener.onAccessPointsChanged();
		}
		ThreadUtils.postOnMainThread(() -> mListener.onAccessPointsChanged());
		}
		}

packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/AccessPointTest.java

+0 −32

Original line number	Diff line number	Diff line
		@@ -112,38 +112,6 @@ public class AccessPointTest {
		assertThat(spans[0].getType()).isEqualTo(TtsSpan.TYPE_TELEPHONE);
		}

		@Test
		public void testCopyAccessPoint_dataShouldMatch() {
		WifiConfiguration configuration = createWifiConfiguration();
		configuration.meteredHint = true;

		NetworkInfo networkInfo =
		new NetworkInfo(ConnectivityManager.TYPE_WIFI, 2, "WIFI", "WIFI_SUBTYPE");
		AccessPoint originalAccessPoint = new AccessPoint(mContext, configuration);
		WifiInfo wifiInfo = new WifiInfo();
		wifiInfo.setSSID(WifiSsid.createFromAsciiEncoded(configuration.SSID));
		wifiInfo.setBSSID(configuration.BSSID);
		originalAccessPoint.update(configuration, wifiInfo, networkInfo);
		AccessPoint copy = new AccessPoint(mContext, originalAccessPoint);

		assertThat(originalAccessPoint.getSsid().toString()).isEqualTo(copy.getSsid().toString());
		assertThat(originalAccessPoint.getBssid()).isEqualTo(copy.getBssid());
		assertThat(originalAccessPoint.getConfig()).isEqualTo(copy.getConfig());
		assertThat(originalAccessPoint.getSecurity()).isEqualTo(copy.getSecurity());
		assertThat(originalAccessPoint.isMetered()).isEqualTo(copy.isMetered());
		assertThat(originalAccessPoint.compareTo(copy) == 0).isTrue();
		}

		@Test
		public void testThatCopyAccessPoint_scanCacheShouldMatch() {
		AccessPoint original = createAccessPointWithScanResultCache();
		assertThat(original.getRssi()).isEqualTo(4);
		AccessPoint copy = new AccessPoint(mContext, createWifiConfiguration());
		assertThat(copy.getRssi()).isEqualTo(AccessPoint.UNREACHABLE_RSSI);
		copy.copyFrom(original);
		assertThat(original.getRssi()).isEqualTo(copy.getRssi());
		}

		@Test
		public void testCompareTo_GivesActiveBeforeInactive() {
		AccessPoint activeAp = new TestAccessPointBuilder(mContext).setActive(true).build();

packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java

+0 −32

Original line number	Diff line number	Diff line
		@@ -382,38 +382,6 @@ public class WifiTrackerTest {
		waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker);
		}

		@Test
		public void testAccessPointListenerSetWhenLookingUpUsingScanResults() {
		ScanResult scanResult = new ScanResult();
		scanResult.level = 123;
		scanResult.BSSID = "bssid-" + 111;
		scanResult.timestamp = SystemClock.elapsedRealtime() * 1000;
		scanResult.capabilities = "";

		WifiTracker tracker = new WifiTracker(
		InstrumentationRegistry.getTargetContext(), null, true, true);

		AccessPoint result = tracker.getCachedOrCreate(
		Collections.singletonList(scanResult), new ArrayList<AccessPoint>());
		assertTrue(result.mAccessPointListener != null);
		}

		@Test
		public void testAccessPointListenerSetWhenLookingUpUsingWifiConfiguration() {
		WifiConfiguration configuration = new WifiConfiguration();
		configuration.SSID = "test123";
		configuration.BSSID="bssid";
		configuration.networkId = 123;
		configuration.allowedKeyManagement = new BitSet();
		configuration.allowedKeyManagement.set(WifiConfiguration.KeyMgmt.WPA_PSK);

		WifiTracker tracker = new WifiTracker(
		InstrumentationRegistry.getTargetContext(), null, true, true);

		AccessPoint result = tracker.getCachedOrCreate(configuration, new ArrayList<AccessPoint>());
		assertTrue(result.mAccessPointListener != null);
		}

		@Test
		public void startAndStopTrackingShouldRegisterAndUnregisterScoreCache()
		throws InterruptedException {