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

Commit 187a8ac9 authored by Eric Lin's avatar Eric Lin
Browse files

Fix display settings persistence with LRU cache. 

Rotation and density settings for external or simulated displays were
not being persisted across reboots. This was caused by an overly
aggressive cleanup mechanism that would remove settings for displays
that were only temporarily disconnected, such as during a device reboot.

This change replaces the ArrayMap in DisplayWindowSettingsProvider with
an LruCache to manage display settings. This aligns with the persistence
strategy used by DisplayManager's DisplayTopologyXmlStore, ensuring that
the settings for the 100 most recently used displays are retained.

When the cache limit is reached, the least recently used setting is
evicted. This prevents the settings file from growing indefinitely while
correctly preserving settings for active and recently used displays
across reboots. The previous removeStaleDisplaySettingsLocked method is
now obsolete and has been removed.

Bug: 397398590
Flag: EXEMPT bug fix
Test: atest WmTests:DisplayWindowSettingsProviderTests
Change-Id: I6bf2a0312403ebda10194bc76fe3e443d50f664b
parent 73ad5173

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

+17 −48
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ 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;
@@ -54,10 +55,7 @@ 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
@@ -73,6 +71,13 @@ 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;
@@ -148,38 +153,6 @@ 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.

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

        protected int mIdentifierType;

        @NonNull

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

        protected final LruCache<String, SettingsEntry> mSettings =

                new LruCache<>(MAX_NUMBER_OF_DISPLAY_SETTINGS);



        ReadableSettings(@NonNull ReadableSettingsStorage settingsStorage) {

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

            if (fileData != null) {

                mIdentifierType = fileData.mIdentifierType;

                mSettings.putAll(fileData.mSettings);

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

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

                }

            }

        }

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


        void onDisplayRemoved(@NonNull DisplayInfo info) {

            final String identifier = getIdentifier(info);

            if (!mSettings.containsKey(identifier)) {

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

                return;

            }

            if (mVirtualDisplayIdentifiers.remove(identifier)
@@ -359,23 +335,16 @@ 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;

            final int size = mSettings.size();

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

                final String identifier = mSettings.keyAt(i);

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

                final String identifier = entry.getKey();

                if (mVirtualDisplayIdentifiers.contains(identifier)) {

                    // Do not write virtual display settings to file.

                    continue;

                }

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

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

            }

            DisplayWindowSettingsProvider.writeSettings(mSettingsStorage, fileData);

        }

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

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

            mCurrentUserId = newUserId;

            mDisplayWindowSettingsProvider.setOverrideSettingsForUser(newUserId);

            mDisplayWindowSettingsProvider.removeStaleDisplaySettingsLocked(this, mRoot);

            mPolicy.setCurrentUserLw(newUserId);

            mKeyguardDisableHandler.setCurrentUser(newUserId);
@@ -5763,9 +5762,6 @@ 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

+41 −45
Original line number Diff line number Diff line
@@ -24,14 +24,11 @@ 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;
@@ -57,7 +54,6 @@ 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;



/**
@@ -73,6 +69,7 @@ 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();
@@ -335,54 +332,53 @@ public class DisplayWindowSettingsProviderTests extends WindowTestsBase { 
    }



    @Test

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

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

        }



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

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

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

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

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

        addDisplayWithDensity(exceedMaxDisplay, 100 + MAX_NUMBER_OF_DISPLAY_SETTINGS);



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

    }



    @Test

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

    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");



        // 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);

        mProvider.getSettings(oldestDisplay);

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

        addDisplayWithDensity(exceedMaxDisplay, 100 + MAX_NUMBER_OF_DISPLAY_SETTINGS);



        provider.removeStaleDisplaySettingsLocked(mWm, mRootWindowContainer);

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

    }



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

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

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

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

    /** 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);

    }



    /**