Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3c71bbcc authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge changes from topic "WifiTrackerRefactor" 

* changes:
  Consolidate all work in the WorkHandler.
  Remove the double lists in WifiTracker.
  Delete the MainHandler and remaining code.
  Remove methods from MainHandler.
parents d067decc c5bbecb3

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

+59 −44
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.
@@ -467,7 +446,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
        }

        builder.append(",metered=").append(isMetered());



        if (WifiTracker.sVerboseLogging) {

        if (isVerboseLoggingEnabled()) {

            builder.append(",rssi=").append(mRssi);

            builder.append(",scan cache size=").append(mScanResults.size());

        }
@@ -546,7 +525,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
        mSpeed = generateAverageSpeedForSsid();



        boolean changed = oldSpeed != mSpeed;

        if(WifiTracker.sVerboseLogging && changed) {

        if(isVerboseLoggingEnabled() && changed) {

            Log.i(TAG, String.format("%s: Set speed to %d", ssid, mSpeed));

        }

        return changed;
@@ -577,7 +556,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
            }

        }

        int speed = count == 0 ? Speed.NONE : totalSpeed / count;

        if (WifiTracker.sVerboseLogging) {

        if (isVerboseLoggingEnabled()) {

            Log.i(TAG, String.format("%s generated fallback speed is: %d", getSsidStr(), speed));

        }

        return roundToClosestSpeedEnum(speed);
@@ -913,7 +892,7 @@ public class AccessPoint implements Comparable<AccessPoint> { 
            }

        }



        if (WifiTracker.sVerboseLogging) {

        if (isVerboseLoggingEnabled()) {

            summary.append(WifiUtils.buildLoggingSummary(this, config));

        }
@@ -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,8 +1312,44 @@ 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() {

        return WifiTracker.sVerboseLogging || Log.isLoggable(TAG, Log.VERBOSE);

    }

}

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

+105 −217

File changed.

Preview size limit exceeded, changes collapsed.

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

+10 −56
Original line number Diff line number Diff line
@@ -356,19 +356,14 @@ public class WifiTrackerTest { 


    private void waitForHandlersToProcessCurrentlyEnqueuedMessages(WifiTracker tracker)

            throws InterruptedException {

        // TODO(sghuman): This should no longer be necessary in a single work handler model



        CountDownLatch workerLatch = new CountDownLatch(1);

        tracker.mWorkHandler.post(() -> {

            workerLatch.countDown();

        });

        assertTrue("Latch timed out while waiting for WorkerHandler",

                workerLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS));



        CountDownLatch mainLatch = new CountDownLatch(1);

        tracker.mMainHandler.post(() -> {

            mainLatch.countDown();

        });

        assertTrue("Latch timed out while waiting for MainHandler",

                mainLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS));

    }



    private void switchToNetwork2(WifiTracker tracker) throws InterruptedException {
@@ -389,38 +384,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 {
@@ -534,7 +497,6 @@ public class WifiTrackerTest { 
        waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker);

    }



    @FlakyTest

    @Test

    public void scoreCacheUpdateScoresShouldChangeSortOrder() throws InterruptedException {

        WifiTracker tracker =  createTrackerWithImmediateBroadcastsAndInjectInitialScanResults();
@@ -634,9 +596,9 @@ public class WifiTrackerTest { 
    public void scoresShouldBeRequestedForNewScanResultOnly()  throws InterruptedException {

        // Scores can be requested together or serially depending on how the scan results are

        // processed.

        mRequestScoresLatch = new CountDownLatch(2);

        mRequestScoresLatch = new CountDownLatch(1);

        WifiTracker tracker = createTrackerWithImmediateBroadcastsAndInjectInitialScanResults();

        mRequestScoresLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS);

        assertTrue(mRequestScoresLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS));

        mRequestedKeys.clear();



        String ssid = "ssid3";
@@ -770,7 +732,7 @@ public class WifiTrackerTest { 
        CountDownLatch ready = new CountDownLatch(1);

        CountDownLatch latch = new CountDownLatch(1);

        CountDownLatch lock = new CountDownLatch(1);

        tracker.mMainHandler.post(() -> {

        tracker.mWorkHandler.post(() -> {

            try {

                ready.countDown();

                lock.await();
@@ -781,12 +743,7 @@ public class WifiTrackerTest { 
        });



        // Enqueue messages

        tracker.mMainHandler.sendEmptyMessage(

                WifiTracker.MainHandler.MSG_ACCESS_POINT_CHANGED);

        tracker.mMainHandler.sendEmptyMessage(

                WifiTracker.MainHandler.MSG_CONNECTED_CHANGED);

        tracker.mMainHandler.sendEmptyMessage(

                WifiTracker.MainHandler.MSG_WIFI_STATE_CHANGED);

        tracker.mWorkHandler.sendEmptyMessage(WifiTracker.WorkHandler.MSG_UPDATE_ACCESS_POINTS);



        try {

            ready.await(); // Make sure we have entered the first message handler
@@ -800,12 +757,9 @@ public class WifiTrackerTest { 
        lock.countDown();

        assertTrue("Latch timed out", latch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS));



        assertThat(tracker.mMainHandler.hasMessages(

                WifiTracker.MainHandler.MSG_ACCESS_POINT_CHANGED)).isFalse();

        assertThat(tracker.mMainHandler.hasMessages(

                WifiTracker.MainHandler.MSG_CONNECTED_CHANGED)).isFalse();

        assertThat(tracker.mMainHandler.hasMessages(

                WifiTracker.MainHandler.MSG_WIFI_STATE_CHANGED)).isFalse();

        assertThat(tracker.mWorkHandler.hasMessages(

                WifiTracker.WorkHandler.MSG_UPDATE_ACCESS_POINTS)).isFalse();

        waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker);



        verifyNoMoreInteractions(mockWifiListener);

    }
@@ -862,7 +816,7 @@ public class WifiTrackerTest { 
        mAccessPointsChangedLatch = new CountDownLatch(1);

        tracker.mReceiver.onReceive(mContext, new Intent(WifiManager.WIFI_STATE_CHANGED_ACTION));



        mAccessPointsChangedLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS);

        assertTrue(mAccessPointsChangedLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS));

        waitForHandlersToProcessCurrentlyEnqueuedMessages(tracker);



        assertThat(tracker.getAccessPoints()).isEmpty();