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

Commit 71f4a82b authored by Sundeep Ghuman's avatar Sundeep Ghuman
Browse files

Remove all pending callbacks in stopTracking. 

This prevents callbacks occurring on the Settings UI thread in the
WifiSettings fragment after its Activity is already destroyed. When
removing pending ACCESS_POINTS_CHANGED_MESSAGES, release the lock.

Also fixed some flakey tests due to threading issues.

Bug: b/37472533
Test: runtest --path
frameworks/base/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java

Change-Id: I764caf52e38b7f7e33c67c7ea3aa3d2301095dca
parent e4964118

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

+30 −9
Original line number Diff line number Diff line
@@ -16,7 +16,6 @@ 
package com.android.settingslib.wifi;



import android.content.BroadcastReceiver;

import android.content.ContentResolver;

import android.content.Context;

import android.content.Intent;

import android.content.IntentFilter;
@@ -91,7 +90,7 @@ public class WifiTracker { 
    private final boolean mIncludeSaved;

    private final boolean mIncludeScans;

    private final boolean mIncludePasspoints;

    private final MainHandler mMainHandler;

    @VisibleForTesting final MainHandler mMainHandler;

    private final WorkHandler mWorkHandler;



    private WifiTrackerNetworkCallback mNetworkCallback;
@@ -335,13 +334,13 @@ public class WifiTracker { 
     */

    public void stopTracking() {

        if (mRegistered) {

            mWorkHandler.removeMessages(WorkHandler.MSG_UPDATE_ACCESS_POINTS);

            mWorkHandler.removeMessages(WorkHandler.MSG_UPDATE_NETWORK_INFO);

            mWorkHandler.removeMessages(WorkHandler.MSG_UPDATE_NETWORK_SCORES);

            mContext.unregisterReceiver(mReceiver);

            mConnectivityManager.unregisterNetworkCallback(mNetworkCallback);

            mRegistered = false;

        }

        mWorkHandler.removePendingMessages();

        mMainHandler.removePendingMessages();



        pauseScanning();



        unregisterAndClearScoreCache();
@@ -749,10 +748,11 @@ public class WifiTracker { 
        }

    }



    private final class MainHandler extends Handler {

        private static final int MSG_CONNECTED_CHANGED = 0;

        private static final int MSG_WIFI_STATE_CHANGED = 1;

        private static final int MSG_ACCESS_POINT_CHANGED = 2;

    @VisibleForTesting

    final class MainHandler extends Handler {

        @VisibleForTesting static final int MSG_CONNECTED_CHANGED = 0;

        @VisibleForTesting static final int MSG_WIFI_STATE_CHANGED = 1;

        @VisibleForTesting static final int MSG_ACCESS_POINT_CHANGED = 2;

        private static final int MSG_RESUME_SCANNING = 3;

        private static final int MSG_PAUSE_SCANNING = 4;
@@ -794,6 +794,19 @@ public class WifiTracker { 
            mInternalAccessPointsWriteLock.close();

            sendEmptyMessage(MSG_ACCESS_POINT_CHANGED);

        }



        void removePendingMessages() {

            // Only release the lock if there was a pending message which would have done the same

            if (mMainHandler.hasMessages(MSG_ACCESS_POINT_CHANGED)) {

                mMainHandler.removeMessages(MSG_ACCESS_POINT_CHANGED);

                mInternalAccessPointsWriteLock.open();

            }



            removeMessages(MSG_CONNECTED_CHANGED);

            removeMessages(MSG_WIFI_STATE_CHANGED);

            removeMessages(MSG_PAUSE_SCANNING);

            removeMessages(MSG_RESUME_SCANNING);

        }

    }



    private final class WorkHandler extends Handler {
@@ -841,6 +854,14 @@ public class WifiTracker { 
                    break;

            }

        }



        private void removePendingMessages() {

            removeMessages(MSG_UPDATE_ACCESS_POINTS);

            removeMessages(MSG_UPDATE_NETWORK_INFO);

            removeMessages(MSG_RESUME);

            removeMessages(MSG_UPDATE_WIFI_STATE);

            removeMessages(MSG_UPDATE_NETWORK_SCORES);

        }

    }



    @VisibleForTesting

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

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


import static com.google.common.truth.Truth.assertThat;

import static com.google.common.truth.Truth.assertWithMessage;



import static org.junit.Assert.assertTrue;

import static org.mockito.Mockito.any;

import static org.mockito.Mockito.when;
@@ -24,7 +25,6 @@ import static org.mockito.Mockito.when; 
import android.content.Context;

import android.net.ConnectivityManager;

import android.net.NetworkInfo;

import android.net.NetworkKey;

import android.net.ScoredNetwork;

import android.net.wifi.ScanResult;

import android.net.wifi.WifiConfiguration;
@@ -40,6 +40,7 @@ import android.support.test.filters.SmallTest; 
import android.support.test.runner.AndroidJUnit4;

import android.text.SpannableString;

import android.text.style.TtsSpan;



import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;

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

+69 −13
Original line number Diff line number Diff line
@@ -20,13 +20,15 @@ import static com.google.common.truth.Truth.assertThat; 
import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertTrue;

import static org.junit.Assert.fail;

import static org.mockito.Mockito.any;

import static org.mockito.Mockito.anyInt;

import static org.mockito.Mockito.atLeast;

import static org.mockito.Mockito.atMost;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.doNothing;

import static org.mockito.Mockito.doThrow;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.verifyNoMoreInteractions;

import static org.mockito.Mockito.when;



import android.content.Context;
@@ -60,7 +62,6 @@ import org.junit.After; 
import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;



import org.mockito.ArgumentCaptor;

import org.mockito.Captor;

import org.mockito.Matchers;
@@ -129,7 +130,7 @@ public class WifiTrackerTest { 
    private Handler mScannerHandler;

    private HandlerThread mMainThread;

    private HandlerThread mWorkerThread;

    private Looper mLooper;

    private Looper mWorkerLooper;

    private Looper mMainLooper;

    private int mOriginalScoringUiSettingValue;
@@ -141,7 +142,7 @@ public class WifiTrackerTest { 


        mWorkerThread = new HandlerThread("TestHandlerWorkerThread");

        mWorkerThread.start();

        mLooper = mWorkerThread.getLooper();

        mWorkerLooper = mWorkerThread.getLooper();

        mMainThread = new HandlerThread("TestHandlerThread");

        mMainThread.start();

        mMainLooper = mMainThread.getLooper();
@@ -264,7 +265,7 @@ public class WifiTrackerTest { 
                new WifiTracker(

                        mContext,

                        mockWifiListener,

                        mLooper,

                        mWorkerLooper,

                        true,

                        true,

                        true,
@@ -349,7 +350,7 @@ public class WifiTrackerTest { 
        scanResult.capabilities = "";



        WifiTracker tracker = new WifiTracker(

                InstrumentationRegistry.getTargetContext(), null, mLooper, true, true);

                InstrumentationRegistry.getTargetContext(), null, mWorkerLooper, true, true);



        AccessPoint result = tracker.getCachedOrCreate(scanResult, new ArrayList<AccessPoint>());

        assertTrue(result.mAccessPointListener != null);
@@ -365,7 +366,7 @@ public class WifiTrackerTest { 
        configuration.allowedKeyManagement.set(WifiConfiguration.KeyMgmt.WPA_PSK);



        WifiTracker tracker = new WifiTracker(

                InstrumentationRegistry.getTargetContext(), null, mLooper, true, true);

                InstrumentationRegistry.getTargetContext(), null, mWorkerLooper, true, true);



        AccessPoint result = tracker.getCachedOrCreate(configuration, new ArrayList<AccessPoint>());

        assertTrue(result.mAccessPointListener != null);
@@ -433,7 +434,8 @@ public class WifiTrackerTest { 
    }



    @Test

    public void startTrackingShouldRequestScoresForCurrentAccessPoints() throws InterruptedException {

    public void startTrackingAfterStopTracking_shouldRequestNewScores()

            throws InterruptedException {

        // Start the tracker and inject the initial scan results and then stop tracking

        WifiTracker tracker =  createTrackerWithImmediateBroadcastsAndInjectInitialScanResults();
@@ -442,6 +444,7 @@ public class WifiTrackerTest { 


        mRequestScoresLatch = new CountDownLatch(1);

        startTracking(tracker);

        tracker.forceUpdate();

        assertTrue("Latch timed out",

                mRequestScoresLatch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS));
@@ -657,18 +660,29 @@ public class WifiTrackerTest { 
        int newRssi = CONNECTED_RSSI + 10;

        WifiInfo info = new WifiInfo(CONNECTED_AP_1_INFO);

        info.setRssi(newRssi);

        when(mockWifiManager.getConnectionInfo()).thenReturn(info);



        CountDownLatch latch = new CountDownLatch(1);



        // Once the new info has been fetched, we need to wait for the access points to be copied

        doAnswer(invocation -> {

                    latch.countDown();

                    mAccessPointsChangedLatch = new CountDownLatch(1);

                    return info;

                }).when(mockWifiManager).getConnectionInfo();



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

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

        assertTrue("New connection info never retrieved",

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

        assertTrue("onAccessPointsChanged never called",

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



        verify(mockWifiManager, atLeast(2)).getConnectionInfo();

        assertThat(tracker.getAccessPoints().get(0).getRssi()).isEqualTo(newRssi);

    }



    @Test

    public void forceUpdateShouldSynchronouslyFetchLatestInformation() throws Exception {

        // TODO(sghuman): Fix flakiness of this test



        when(mockWifiManager.getConnectionInfo()).thenReturn(CONNECTED_AP_1_INFO);



        WifiConfiguration configuration = new WifiConfiguration();
@@ -682,7 +696,6 @@ public class WifiTrackerTest { 
        networkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTED, "connected", "test");

        when(mockConnectivityManager.getNetworkInfo(any(Network.class))).thenReturn(networkInfo);





        WifiTracker tracker = createMockedWifiTracker();

        startTracking(tracker);

        tracker.forceUpdate();
@@ -691,4 +704,47 @@ public class WifiTrackerTest { 
        assertThat(tracker.getAccessPoints().size()).isEqualTo(2);

        assertThat(tracker.getAccessPoints().get(0).isActive()).isTrue();

    }



    @Test

    public void stopTrackingShouldRemoveWifiListenerCallbacks() throws Exception {

        WifiTracker tracker = createMockedWifiTracker();

        startTracking(tracker);



        CountDownLatch latch = new CountDownLatch(1);

        CountDownLatch lock = new CountDownLatch(1);

        tracker.mMainHandler.post(() -> {

            try {

                lock.await();

                latch.countDown();

            } catch (InterruptedException e) {

                fail("Interrupted Exception while awaiting lock release: " + e);

            }

        });



        // 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.stopTracking();



        verify(mockWifiListener, atMost(1)).onAccessPointsChanged();

        verify(mockWifiListener, atMost(1)).onConnectedChanged();

        verify(mockWifiListener, atMost(1)).onWifiStateChanged(anyInt());



        lock.countDown();

        assertTrue(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();



        verifyNoMoreInteractions(mockWifiListener);

    }

}
 
 No newline at end of file