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

Commit b9f9e64d authored by Charles Chen's avatar Charles Chen
Browse files

Fix leakage issue in HighFreshRateBlacklist in WmTests 

Also refactor HighRefreshRateBlacklisTest a bit.

Fixes: 144426453
Test: atest WmTests
Change-Id: Ib97c4292bb7287f3e1f488c65259ab99810280b0
parent bff5f30f

services/core/java/com/android/server/wm/HighRefreshRateBlacklist.java

+25 −3
Original line number Diff line number Diff line
@@ -41,26 +41,38 @@ class HighRefreshRateBlacklist { 
    private final String[] mDefaultBlacklist;

    private final Object mLock = new Object();



    private DeviceConfigInterface mDeviceConfig;

    private OnPropertiesChangedListener mListener = new OnPropertiesChangedListener();



    static HighRefreshRateBlacklist create(@NonNull Resources r) {

        return new HighRefreshRateBlacklist(r, new DeviceConfigInterface() {

            @Override

            public @Nullable String getProperty(@NonNull String namespace, @NonNull String name) {

                return DeviceConfig.getProperty(namespace, name);

            }



            @Override

            public void addOnPropertiesChangedListener(@NonNull String namespace,

                    @NonNull Executor executor,

                    @NonNull DeviceConfig.OnPropertiesChangedListener listener) {

                DeviceConfig.addOnPropertiesChangedListener(namespace, executor, listener);

            }



            @Override

            public void removePropertiesChangedListener(

                    DeviceConfig.OnPropertiesChangedListener listener) {

                DeviceConfig.removeOnPropertiesChangedListener(listener);

            }

        });

    }



    @VisibleForTesting

    HighRefreshRateBlacklist(Resources r, DeviceConfigInterface deviceConfig) {

        mDefaultBlacklist = r.getStringArray(R.array.config_highRefreshRateBlacklist);

        deviceConfig.addOnPropertiesChangedListener(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,

                BackgroundThread.getExecutor(), new OnPropertiesChangedListener());

        final String property = deviceConfig.getProperty(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,

        mDeviceConfig = deviceConfig;

        mDeviceConfig.addOnPropertiesChangedListener(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,

                BackgroundThread.getExecutor(), mListener);

        final String property = mDeviceConfig.getProperty(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,

                KEY_HIGH_REFRESH_RATE_BLACKLIST);

        updateBlacklist(property);

    }
@@ -101,10 +113,20 @@ class HighRefreshRateBlacklist { 
        }

    }



    /** Used to prevent WmTests leaking issues. */

    @VisibleForTesting

    void dispose() {

        mDeviceConfig.removePropertiesChangedListener(mListener);

        mDeviceConfig = null;

        mBlacklistedPackages.clear();

    }



    interface DeviceConfigInterface {

        @Nullable String getProperty(@NonNull String namespace, @NonNull String name);

        void addOnPropertiesChangedListener(@NonNull String namespace, @NonNull Executor executor,

                @NonNull DeviceConfig.OnPropertiesChangedListener listener);

        void removePropertiesChangedListener(

                @NonNull DeviceConfig.OnPropertiesChangedListener listener);

    }



    private class OnPropertiesChangedListener implements DeviceConfig.OnPropertiesChangedListener {

services/tests/wmtests/src/com/android/server/wm/HighRefreshRateBlacklistTest.java

+51 −32
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ import androidx.test.filters.SmallTest; 
import com.android.internal.R;

import com.android.server.wm.HighRefreshRateBlacklist.DeviceConfigInterface;



import org.junit.After;

import org.junit.Test;



import java.util.ArrayList;
@@ -50,67 +51,79 @@ import java.util.concurrent.TimeUnit; 
@SmallTest

@Presubmit

public class HighRefreshRateBlacklistTest {

    private static final String APP1 = "com.android.sample1";

    private static final String APP2 = "com.android.sample2";

    private static final String APP3 = "com.android.sample3";



    private HighRefreshRateBlacklist mBlacklist;



    @After

    public void tearDown() {

        mBlacklist.dispose();

    }



    @Test

    public void testDefaultBlacklist() {

        final Resources r = createResources("com.android.sample1", "com.android.sample2");

        HighRefreshRateBlacklist blacklist =

                new HighRefreshRateBlacklist(r, new FakeDeviceConfigInterface());

        assertTrue(blacklist.isBlacklisted("com.android.sample1"));

        assertTrue(blacklist.isBlacklisted("com.android.sample2"));

        assertFalse(blacklist.isBlacklisted("com.android.sample3"));

        final Resources r = createResources(APP1, APP2);

        mBlacklist = new HighRefreshRateBlacklist(r, new FakeDeviceConfigInterface());



        assertTrue(mBlacklist.isBlacklisted(APP1));

        assertTrue(mBlacklist.isBlacklisted(APP2));

        assertFalse(mBlacklist.isBlacklisted(APP3));

    }



    @Test

    public void testNoDefaultBlacklist() {

        final Resources r = createResources();

        HighRefreshRateBlacklist blacklist =

                new HighRefreshRateBlacklist(r, new FakeDeviceConfigInterface());

        assertFalse(blacklist.isBlacklisted("com.android.sample1"));

        mBlacklist = new HighRefreshRateBlacklist(r, new FakeDeviceConfigInterface());



        assertFalse(mBlacklist.isBlacklisted(APP1));

    }



    @Test

    public void testDefaultBlacklistIsOverriddenByDeviceConfig() {

        final Resources r = createResources("com.android.sample1");

        final Resources r = createResources(APP1);

        final FakeDeviceConfigInterface config = new FakeDeviceConfigInterface();

        config.setBlacklist("com.android.sample2,com.android.sample3");

        HighRefreshRateBlacklist blacklist = new HighRefreshRateBlacklist(r, config);

        assertFalse(blacklist.isBlacklisted("com.android.sample1"));

        assertTrue(blacklist.isBlacklisted("com.android.sample2"));

        assertTrue(blacklist.isBlacklisted("com.android.sample3"));

        config.setBlacklist(APP2 + "," + APP3);

        mBlacklist = new HighRefreshRateBlacklist(r, config);



        assertFalse(mBlacklist.isBlacklisted(APP1));

        assertTrue(mBlacklist.isBlacklisted(APP2));

        assertTrue(mBlacklist.isBlacklisted(APP3));

    }



    @Test

    public void testDefaultBlacklistIsOverriddenByEmptyDeviceConfig() {

        final Resources r = createResources("com.android.sample1");

        final Resources r = createResources(APP1);

        final FakeDeviceConfigInterface config = new FakeDeviceConfigInterface();

        config.setBlacklist("");

        HighRefreshRateBlacklist blacklist = new HighRefreshRateBlacklist(r, config);

        assertFalse(blacklist.isBlacklisted("com.android.sample1"));

        mBlacklist = new HighRefreshRateBlacklist(r, config);



        assertFalse(mBlacklist.isBlacklisted(APP1));

    }



    @Test

    public void testDefaultBlacklistIsOverriddenByDeviceConfigUpdate() {

        final Resources r = createResources("com.android.sample1");

        final Resources r = createResources(APP1);

        final FakeDeviceConfigInterface config = new FakeDeviceConfigInterface();

        HighRefreshRateBlacklist blacklist = new HighRefreshRateBlacklist(r, config);

        mBlacklist = new HighRefreshRateBlacklist(r, config);



        // First check that the default blacklist is in effect

        assertTrue(blacklist.isBlacklisted("com.android.sample1"));

        assertFalse(blacklist.isBlacklisted("com.android.sample2"));

        assertFalse(blacklist.isBlacklisted("com.android.sample3"));

        assertTrue(mBlacklist.isBlacklisted(APP1));

        assertFalse(mBlacklist.isBlacklisted(APP2));

        assertFalse(mBlacklist.isBlacklisted(APP3));



        //  Then confirm that the DeviceConfig list has propagated and taken effect.

        config.setBlacklist("com.android.sample2,com.android.sample3");

        assertFalse(blacklist.isBlacklisted("com.android.sample1"));

        assertTrue(blacklist.isBlacklisted("com.android.sample2"));

        assertTrue(blacklist.isBlacklisted("com.android.sample3"));

        config.setBlacklist(APP2 + "," + APP3);

        assertFalse(mBlacklist.isBlacklisted(APP1));

        assertTrue(mBlacklist.isBlacklisted(APP2));

        assertTrue(mBlacklist.isBlacklisted(APP3));



        //  Finally make sure we go back to the default list if the DeviceConfig gets deleted.

        config.setBlacklist(null);

        assertTrue(blacklist.isBlacklisted("com.android.sample1"));

        assertFalse(blacklist.isBlacklisted("com.android.sample2"));

        assertFalse(blacklist.isBlacklisted("com.android.sample3"));

        assertTrue(mBlacklist.isBlacklisted(APP1));

        assertFalse(mBlacklist.isBlacklisted(APP2));

        assertFalse(mBlacklist.isBlacklisted(APP3));

    }



    private Resources createResources(String... defaultBlacklist) {
@@ -120,8 +133,7 @@ public class HighRefreshRateBlacklistTest { 
        return r;

    }





    class FakeDeviceConfigInterface implements DeviceConfigInterface {

    private static class FakeDeviceConfigInterface implements DeviceConfigInterface {

        private List<Pair<DeviceConfig.OnPropertiesChangedListener, Executor>> mListeners =

                new ArrayList<>();

        private String mBlacklist;
@@ -147,6 +159,13 @@ public class HighRefreshRateBlacklistTest { 
            mListeners.add(new Pair<>(listener, executor));

        }



        @Override

        public void removePropertiesChangedListener(

                DeviceConfig.OnPropertiesChangedListener listener) {

            // Don't need to implement since we don't invoke real DeviceConfig and we clear

            // HighRefreshRateBlacklist#mDeviceConfig in HighRefreshRateBlacklist#dispose.

        }



        void setBlacklist(String blacklist) {

            mBlacklist = blacklist;

            CountDownLatch latch = new CountDownLatch(mListeners.size());

services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRule.java

+1 −0
Original line number Diff line number Diff line
@@ -303,6 +303,7 @@ public class SystemServicesTestRule implements TestRule { 
        // a static object, so we need to clean it up in tearDown(), even though we didn't set up

        // in tests.

        DeviceConfig.removeOnPropertiesChangedListener(mWmService.mPropertiesChangedListener);

        mWmService.mHighRefreshRateBlacklist.dispose();



        waitUntilWindowManagerHandlersIdle();

        // Needs to explicitly dispose current static threads because there could be messages