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

Commit 2ff40d9d authored by Eric Lin's avatar Eric Lin Committed by Android (Google) Code Review
Browse files

Merge "Revert^2 "Fix display settings persistence with LRU cache."" into main

parents efd400c3 c10259dc

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

+19 −50
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);
@@ -260,7 +234,7 @@ class DisplayWindowSettingsProvider implements SettingsProvider { 
                return settings;

            }

            // Else, fall back to the display name.

            if ((settings = mSettings.get(info.name)) != null) {

            if (info.name != null && (settings = mSettings.get(info.name)) != null) {

                // Found an entry stored with old identifier.

                mSettings.remove(info.name);

                mSettings.put(identifier, settings);
@@ -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());

                }

            }

        }

    }
@@ -314,7 +290,7 @@ class DisplayWindowSettingsProvider implements SettingsProvider { 
                return settings;

            }

            // Else, fall back to the display name.

            if ((settings = mSettings.get(info.name)) != null) {

            if (info.name != null && (settings = mSettings.get(info.name)) != null) {

                // Found an entry stored with old identifier.

                mSettings.remove(info.name);

                mSettings.put(identifier, settings);
@@ -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

+51 −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,63 @@ 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 testUpdateOverrideSettings_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 testUpdateOverrideSettings_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");



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

    }



    @Test

    public void testUpdateOverrideSettings_withNullDisplayName_addsDisplaySettingsSucceeds() {

        final DisplayInfo displayInfoWithNullName = createDisplayInfo("test_id", null /* name */);



        provider.removeStaleDisplaySettingsLocked(mWm, mRootWindowContainer);

        addDisplayWithDensity(displayInfoWithNullName, 123);



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

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

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

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

        assertEquals("Settings should be created even with a null display name.", 123,

                mProvider.getSettings(displayInfoWithNullName).mForcedDensity);

    }



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

    }



    /**