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

Commit c73f4eda authored by Eric Lin's avatar Eric Lin
Browse files

Fix display settings loss: consider all possible DisplayInfo. 

DisplayWindowSettingsProvider now considers all possible DisplayInfo
for each logical display when removing stale display settings. This
ensures that settings for all displays, such as inner and outer displays
on foldables, are preserved while cleaning up outdated settings linked
to inactive displays.

Fix: 353661712
Test: atest WmTests:DisplayWindowSettingsProviderTests
Flag: EXEMPT bugfix
Change-Id: Iddb7c24f8f621aadf481605512e0b67816e9f70d
parent 7cbbbdd7

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

+24 −6
Original line number Diff line number Diff line
@@ -55,8 +55,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
@@ -152,19 +154,35 @@ class DisplayWindowSettingsProvider implements SettingsProvider { 


    /**

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

     * This is necessary because displays can be dynamically added or removed during

     * the system's lifecycle (e.g., user switch, system server restart).

     * <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 root The root window container used to obtain the currently active displays.

     * @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 removeStaleDisplaySettings(@NonNull RootWindowContainer root) {

    void removeStaleDisplaySettingsLocked(@NonNull WindowManagerService wms,

            @NonNull RootWindowContainer root) {

        if (!Flags.perUserDisplayWindowSettings()) {

            return;

        }

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

        final Consumer<DisplayInfo> addDisplayIdentifier =

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

        root.forAllDisplays(dc -> {

            final String identifier = mOverrideSettings.getIdentifier(dc.getDisplayInfo());

            displayIdentifiers.add(identifier);

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

    }

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

+1 −1
Original line number Diff line number Diff line
@@ -57,7 +57,7 @@ public class PossibleDisplayInfoMapper { 
     * Returns, for the given displayId, a list of unique display infos. List contains each

     * supported device state.

     * <p>List contents are guaranteed to be unique, but returned as a list rather than a set to

     * minimize copies needed to make an iteraable data structure.

     * minimize copies needed to make an iterable data structure.

     */

    public List<DisplayInfo> getPossibleDisplayInfos(int displayId) {

        // Update display infos before returning, since any cached values would have been removed

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

+2 −2
Original line number Diff line number Diff line
@@ -3750,7 +3750,7 @@ public class WindowManagerService extends IWindowManager.Stub 
            }

            mCurrentUserId = newUserId;

            mDisplayWindowSettingsProvider.setOverrideSettingsForUser(newUserId);

            mDisplayWindowSettingsProvider.removeStaleDisplaySettings(mRoot);

            mDisplayWindowSettingsProvider.removeStaleDisplaySettingsLocked(this, mRoot);

            mPolicy.setCurrentUserLw(newUserId);

            mKeyguardDisableHandler.setCurrentUser(newUserId);
@@ -5491,7 +5491,7 @@ public class WindowManagerService extends IWindowManager.Stub 
            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.removeStaleDisplaySettings(mRoot);

            mDisplayWindowSettingsProvider.removeStaleDisplaySettingsLocked(this, mRoot);

        }

    }

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

+50 −8
Original line number Diff line number Diff line
@@ -24,12 +24,15 @@ 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.junit.Assume.assumeTrue;

import static org.mockito.ArgumentMatchers.eq;

import static org.testng.Assert.assertFalse;



import android.annotation.Nullable;
@@ -58,6 +61,7 @@ import java.io.IOException; 
import java.io.InputStream;

import java.io.OutputStream;

import java.nio.charset.StandardCharsets;

import java.util.List;

import java.util.function.Consumer;



/**
@@ -352,20 +356,58 @@ public class DisplayWindowSettingsProviderTests extends WindowTestsBase { 
    }



    @Test

    public void testRemovesStaleDisplaySettings() {

    public void testRemovesStaleDisplaySettings_defaultDisplay_removesStaleDisplaySettings() {

        assumeTrue(com.android.window.flags.Flags.perUserDisplayWindowSettings());



        final DisplayWindowSettingsProvider provider =

                new DisplayWindowSettingsProvider(mDefaultVendorSettingsStorage,

                        mOverrideSettingsStorage);

        final DisplayInfo displayInfo = mSecondaryDisplay.getDisplayInfo();

        updateOverrideSettings(provider, displayInfo, settings -> settings.mForcedDensity = 356);

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



        provider.removeStaleDisplaySettings(mRootWindowContainer);

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



        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 testRemovesStaleDisplaySettings_displayNotInLayout_keepsDisplaySettings() {

        assumeTrue(com.android.window.flags.Flags.perUserDisplayWindowSettings());



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



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



        provider.removeStaleDisplaySettingsLocked(mWm, mRootWindowContainer);



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

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

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

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

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

    }



    /**