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

Commit 029845d4 authored by Priyanka Advani (xWF)'s avatar Priyanka Advani (xWF) Committed by Android (Google) Code Review
Browse files

Revert "Fix display settings persistence with LRU cache." 

This reverts commit 187a8ac9.

Reason for revert: Droidmonitor created revert due to b/422386919. Will be verifying through ABTD before submission.

Bug: 397398590
Fix: 422386919
Change-Id: Ia806c29c8bb240c3c72d876e96a28e34c95be810
parent 187a8ac9

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

+48 −17
Original line number Diff line number Diff line
@@ -34,7 +34,6 @@ import android.os.Environment; 
import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.AtomicFile;

import android.util.LruCache;

import android.util.Slog;

import android.util.Xml;

import android.view.DisplayAddress;
@@ -55,7 +54,10 @@ import java.io.FileOutputStream; 
import java.io.IOException;

import java.io.InputStream;

import java.io.OutputStream;

import java.util.List;

import java.util.Map;

import java.util.Set;

import java.util.function.Consumer;



/**

 * Implementation of {@link SettingsProvider} that reads the base settings provided in a display
@@ -71,13 +73,6 @@ class DisplayWindowSettingsProvider implements SettingsProvider { 
    private static final String DATA_DISPLAY_SETTINGS_FILE_PATH = "system/display_settings.xml";

    private static final String VENDOR_DISPLAY_SETTINGS_FILE_PATH = "etc/display_settings.xml";

    private static final String WM_DISPLAY_COMMIT_TAG = "wm-displays";

    /**

     * Maximum number of display settings entries cached in LruCache. When limit is reached,

     * least recently used entries are evicted instead of proactively removing stale settings

     * from dynamic display changes (user switching, system restarts). Aligns with DisplayTopology's

     * LRU approach using DisplayTopologyXmlStore#MAX_NUMBER_OF_TOPOLOGIES.

     */

    private static final int MAX_NUMBER_OF_DISPLAY_SETTINGS = 100;



    private static final int IDENTIFIER_UNIQUE_ID = 0;

    private static final int IDENTIFIER_PORT = 1;
@@ -153,6 +148,38 @@ class DisplayWindowSettingsProvider implements SettingsProvider { 
        setOverrideSettingsStorage(new AtomicFileStorage(settingsFile));

    }



    /**

     * Removes display override settings that are no longer associated with active displays.

     * <p>

     * This cleanup process is essential due to the dynamic nature of displays, which can

     * be added or removed during various system events such as user switching or

     * system server restarts.

     *

     * @param wms  the WindowManagerService instance for retrieving all possible {@link DisplayInfo}

     *             for the given logical display.

     * @param root the root window container used to obtain the currently active displays.

     */

    void removeStaleDisplaySettingsLocked(@NonNull WindowManagerService wms,

            @NonNull RootWindowContainer root) {

        final Set<String> displayIdentifiers = new ArraySet<>();

        final Consumer<DisplayInfo> addDisplayIdentifier =

                displayInfo -> displayIdentifiers.add(mOverrideSettings.getIdentifier(displayInfo));

        root.forAllDisplays(dc -> {

            // Begin with the current display's information. Note that the display layout of the

            // current device state might not include this display (e.g., external or virtual

            // displays), resulting in empty possible display info.

            addDisplayIdentifier.accept(dc.getDisplayInfo());



            // Then, add all possible display information for this display if available.

            final List<DisplayInfo> displayInfos = wms.getPossibleDisplayInfoLocked(dc.mDisplayId);

            final int size = displayInfos.size();

            for (int i = 0; i < size; i++) {

                addDisplayIdentifier.accept(displayInfos.get(i));

            }

        });

        mOverrideSettings.removeStaleDisplaySettings(displayIdentifiers);

    }



    /**

     * Overrides the storage that should be used to save override settings.

     *
@@ -218,8 +245,7 @@ class DisplayWindowSettingsProvider implements SettingsProvider { 
        @DisplayIdentifierType

        protected int mIdentifierType;

        @NonNull

        protected final LruCache<String, SettingsEntry> mSettings =

                new LruCache<>(MAX_NUMBER_OF_DISPLAY_SETTINGS);

        protected final ArrayMap<String, SettingsEntry> mSettings = new ArrayMap<>();



        ReadableSettings(@NonNull ReadableSettingsStorage settingsStorage) {

            loadSettings(settingsStorage);
@@ -259,9 +285,7 @@ class DisplayWindowSettingsProvider implements SettingsProvider { 
            FileData fileData = readSettings(settingsStorage);

            if (fileData != null) {

                mIdentifierType = fileData.mIdentifierType;

                for (final Map.Entry<String, SettingsEntry> entry : fileData.mSettings.entrySet()) {

                    mSettings.put(entry.getKey(), entry.getValue());

                }

                mSettings.putAll(fileData.mSettings);

            }

        }

    }
@@ -318,7 +342,7 @@ class DisplayWindowSettingsProvider implements SettingsProvider { 


        void onDisplayRemoved(@NonNull DisplayInfo info) {

            final String identifier = getIdentifier(info);

            if (mSettings.get(identifier) == null) {

            if (!mSettings.containsKey(identifier)) {

                return;

            }

            if (mVirtualDisplayIdentifiers.remove(identifier)
@@ -335,16 +359,23 @@ class DisplayWindowSettingsProvider implements SettingsProvider { 
            mVirtualDisplayIdentifiers.remove(identifier);

        }



        void removeStaleDisplaySettings(@NonNull Set<String> currentDisplayIdentifiers) {

            if (mSettings.retainAll(currentDisplayIdentifiers)) {

                writeSettings();

            }

        }



        private void writeSettings() {

            final FileData fileData = new FileData();

            fileData.mIdentifierType = mIdentifierType;

            for (final Map.Entry<String, SettingsEntry> entry : mSettings.snapshot().entrySet()) {

                final String identifier = entry.getKey();

            final int size = mSettings.size();

            for (int i = 0; i < size; i++) {

                final String identifier = mSettings.keyAt(i);

                if (mVirtualDisplayIdentifiers.contains(identifier)) {

                    // Do not write virtual display settings to file.

                    continue;

                }

                fileData.mSettings.put(identifier, entry.getValue());

                fileData.mSettings.put(identifier, mSettings.get(identifier));

            }

            DisplayWindowSettingsProvider.writeSettings(mSettingsStorage, fileData);

        }

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

+4 −0
Original line number Diff line number Diff line
@@ -3884,6 +3884,7 @@ public class WindowManagerService extends IWindowManager.Stub 
            }

            mCurrentUserId = newUserId;

            mDisplayWindowSettingsProvider.setOverrideSettingsForUser(newUserId);

            mDisplayWindowSettingsProvider.removeStaleDisplaySettingsLocked(this, mRoot);

            mPolicy.setCurrentUserLw(newUserId);

            mKeyguardDisableHandler.setCurrentUser(newUserId);
@@ -5762,6 +5763,9 @@ public class WindowManagerService extends IWindowManager.Stub 
            // DisplayWindowSettings are applied. In addition, wide-color/hdr/isTouchDevice also

            // affect the Configuration.

            mRoot.forAllDisplays(DisplayContent::reconfigureDisplayLocked);

            // Per-user display settings may leave outdated settings after user switches, especially

            // during reboots starting with the default user without setCurrentUser called.

            mDisplayWindowSettingsProvider.removeStaleDisplaySettingsLocked(this, mRoot);

        }

    }

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

+45 −41
Original line number Diff line number Diff line
@@ -24,11 +24,14 @@ import static android.view.WindowManager.DISPLAY_IME_POLICY_LOCAL; 


import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;



import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;



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



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertNotEquals;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.eq;

import static org.testng.Assert.assertFalse;



import android.annotation.Nullable;
@@ -54,6 +57,7 @@ import java.io.ByteArrayInputStream; 
import java.io.File;

import java.io.InputStream;

import java.nio.charset.StandardCharsets;

import java.util.List;

import java.util.function.Consumer;



/**
@@ -69,7 +73,6 @@ import java.util.function.Consumer; 
public class DisplayWindowSettingsProviderTests extends WindowTestsBase {

    private static final int DISPLAY_PORT = 0xFF;

    private static final long DISPLAY_MODEL = 0xEEEEEEEEL;

    private static final int MAX_NUMBER_OF_DISPLAY_SETTINGS = 100;



    private static final File TEST_FOLDER = getInstrumentation().getTargetContext().getCacheDir();
@@ -332,53 +335,54 @@ public class DisplayWindowSettingsProviderTests extends WindowTestsBase { 
    }



    @Test

    public void testOverrideSettings_whenExceedingCapacity_forgetsOldestDisplaySettings() {

        for (int i = 0; i < MAX_NUMBER_OF_DISPLAY_SETTINGS; i++) {

            final DisplayInfo info = createDisplayInfo("test_id_" + i, "test_display_" + i);

            addDisplayWithDensity(info, 100 + i);

        }



        final DisplayInfo exceedMaxDisplay = createDisplayInfo("id_exceed", "display_exceed");

        addDisplayWithDensity(exceedMaxDisplay, 100 + MAX_NUMBER_OF_DISPLAY_SETTINGS);

    public void testRemovesStaleDisplaySettings_defaultDisplay_removesStaleDisplaySettings() {

        // Write density setting for second display then remove it.

        final DisplayWindowSettingsProvider provider = new DisplayWindowSettingsProvider(

                mDefaultVendorSettingsStorage, mOverrideSettingsStorage);

        final DisplayInfo secDisplayInfo = mSecondaryDisplay.getDisplayInfo();

        updateOverrideSettings(provider, secDisplayInfo, setting -> setting.mForcedDensity = 356);

        mRootWindowContainer.removeChild(mSecondaryDisplay);



        // Write density setting for inner and outer default display.

        final DisplayInfo innerDisplayInfo = mPrimaryDisplay.getDisplayInfo();

        final DisplayInfo outerDisplayInfo = new DisplayInfo(secDisplayInfo);

        outerDisplayInfo.displayId = mPrimaryDisplay.mDisplayId;

        outerDisplayInfo.uniqueId = "TEST_OUTER_DISPLAY_" + System.currentTimeMillis();

        updateOverrideSettings(provider, innerDisplayInfo, setting -> setting.mForcedDensity = 490);

        updateOverrideSettings(provider, outerDisplayInfo, setting -> setting.mForcedDensity = 420);

        final List<DisplayInfo> possibleDisplayInfos = List.of(innerDisplayInfo, outerDisplayInfo);

        doReturn(possibleDisplayInfos)

                .when(mWm).getPossibleDisplayInfoLocked(eq(innerDisplayInfo.displayId));



        provider.removeStaleDisplaySettingsLocked(mWm, mRootWindowContainer);



        assertEquals("Stored settings count should be capped at max capacity",

                MAX_NUMBER_OF_DISPLAY_SETTINGS, mProvider.getOverrideSettingsSize());

        final DisplayInfo evictedDisplay = createDisplayInfo("test_id_0", "test_display_0");

        assertEquals("Forgotten entry should have default density",

                0, mProvider.getSettings(evictedDisplay).mForcedDensity);

        assertThat(mOverrideSettingsStorage.wasWriteSuccessful()).isTrue();

        assertThat(provider.getOverrideSettingsSize()).isEqualTo(2);

        assertThat(provider.getOverrideSettings(innerDisplayInfo).mForcedDensity).isEqualTo(490);

        assertThat(provider.getOverrideSettings(outerDisplayInfo).mForcedDensity).isEqualTo(420);

    }



    @Test

    public void testOverrideSettings_onAccessingOldEntry_preventsPurge() {

        for (int i = 0; i < MAX_NUMBER_OF_DISPLAY_SETTINGS; i++) {

            final DisplayInfo info = createDisplayInfo("test_id_" + i, "test_display_" + i);

            addDisplayWithDensity(info, 100 + i);

        }

        final DisplayInfo oldestDisplay = createDisplayInfo("test_id_0", "test_display_0");

        final DisplayInfo nextOldestDisplay = createDisplayInfo("test_id_1", "test_display_1");

    public void testRemovesStaleDisplaySettings_displayNotInLayout_keepsDisplaySettings() {

        // Write density setting for primary display.

        final DisplayWindowSettingsProvider provider = new DisplayWindowSettingsProvider(

                mDefaultVendorSettingsStorage, mOverrideSettingsStorage);

        final DisplayInfo primDisplayInfo = mPrimaryDisplay.getDisplayInfo();

        updateOverrideSettings(provider, primDisplayInfo, setting -> setting.mForcedDensity = 420);



        mProvider.getSettings(oldestDisplay);

        final DisplayInfo exceedMaxDisplay = createDisplayInfo("id_exceed", "display_exceed");

        addDisplayWithDensity(exceedMaxDisplay, 100 + MAX_NUMBER_OF_DISPLAY_SETTINGS);

        // Add a virtual display and write density setting for it.

        final DisplayInfo virtDisplayInfo = new DisplayInfo(primDisplayInfo);

        virtDisplayInfo.uniqueId = "TEST_VIRTUAL_DISPLAY_" + System.currentTimeMillis();

        createNewDisplay(virtDisplayInfo);

        waitUntilHandlersIdle();  // Wait until unfrozen after a display is added.

        updateOverrideSettings(provider, virtDisplayInfo, setting -> setting.mForcedDensity = 490);



        assertEquals("Recently accessed entry should not be purged",

                100, mProvider.getSettings(oldestDisplay).mForcedDensity);

        assertEquals("The oldest un-accessed entry should be purged",

                0, mProvider.getSettings(nextOldestDisplay).mForcedDensity);

    }

        provider.removeStaleDisplaySettingsLocked(mWm, mRootWindowContainer);



    /** Helper method to create a DisplayInfo object with specific identifiers. */

    private static DisplayInfo createDisplayInfo(String uniqueId, String name) {

        final DisplayInfo displayInfo = new DisplayInfo();

        displayInfo.uniqueId = uniqueId;

        displayInfo.name = name;

        return displayInfo;

    }



    /** Helper method to add a display with a specific density setting. */

    private void addDisplayWithDensity(DisplayInfo displayInfo, int density) {

        updateOverrideSettings(mProvider, displayInfo,

                settings -> settings.mForcedDensity = density);

        assertThat(mOverrideSettingsStorage.wasWriteSuccessful()).isTrue();

        assertThat(provider.getOverrideSettingsSize()).isEqualTo(2);

        assertThat(provider.getOverrideSettings(primDisplayInfo).mForcedDensity).isEqualTo(420);

        assertThat(provider.getOverrideSettings(virtDisplayInfo).mForcedDensity).isEqualTo(490);

    }



    /**