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

Commit 5bf4530d authored by Charles Chen's avatar Charles Chen Committed by Android (Google) Code Review
Browse files

Merge "Fix leakage issue in HighFreshRateBlacklist in WmTests"

parents 9779372e b9f9e64d
Loading
Loading
Loading
Loading
+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 {
+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());
+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