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

Commit a69ad6b7 authored by Roy Chou's avatar Roy Chou
Browse files

fix(magnification): cleanup cached instance for removed display 

When new displays added, for some types of display the sysui magnification
would create corresponding MagnificationSettingsController or
MagnificationModeSwitch for them. Once the instances are created,
they'll live in the sysui lifecycle, even after the specific displays
removed.

Therefore, we add callback in MagnificationImpl, then when a display removed,
it can get notified and do the cleanup for related cached instances.

Bug: 437868750
Flag: com.android.systemui.cleanup_instances_when_display_removed
Test: manually try with system heap dump
      atest com.android.systemui.accessibility
Change-Id: Ifa69c3693f8f9b07dd1ae734d04514a3e0060265
parent 928742d4

packages/SystemUI/aconfig/accessibility.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -3,6 +3,16 @@ container: "system" 


# NOTE: Keep alphabetized to help limit merge conflicts from multiple simultaneous editors.



flag {

    name: "cleanup_instances_when_display_removed"

    namespace: "accessibility"

    description: "When a specific display is removed, cleanup the cached instances related to it."

    bug: "437868750"

    metadata {

        purpose: PURPOSE_BUGFIX

    }

}



flag {

    name: "floating_menu_drag_to_hide"

    namespace: "accessibility"

packages/SystemUI/multivalentTests/src/com/android/systemui/accessibility/ModeSwitchesControllerTest.java

+51 −1
Original line number Diff line number Diff line
@@ -16,10 +16,16 @@ 


package com.android.systemui.accessibility;



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



import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



import android.content.pm.ActivityInfo;

import android.hardware.display.DisplayManager;

import android.platform.test.annotations.DisableFlags;

import android.platform.test.annotations.EnableFlags;

import android.provider.Settings;

import android.testing.TestableLooper;

import android.view.Display;
@@ -29,6 +35,7 @@ import android.view.WindowManager; 
import androidx.test.ext.junit.runners.AndroidJUnit4;

import androidx.test.filters.SmallTest;



import com.android.systemui.Flags;

import com.android.systemui.SysuiTestCase;



import org.junit.After;
@@ -45,6 +52,10 @@ import org.mockito.MockitoAnnotations; 
@TestableLooper.RunWithLooper(setAsMainLooper = true)

public class ModeSwitchesControllerTest extends SysuiTestCase {



    @Mock

    private DisplayManager mDisplayManager;



    private Display mDisplay;

    private FakeSwitchSupplier mSupplier;

    private MagnificationModeSwitch mModeSwitch;

    private ModeSwitchesController mModeSwitchesController;
@@ -56,7 +67,12 @@ public class ModeSwitchesControllerTest extends SysuiTestCase { 
    @Before

    public void setUp() {

        MockitoAnnotations.initMocks(this);

        mSupplier = new FakeSwitchSupplier(mContext.getSystemService(DisplayManager.class));



        mDisplay = mContext.getSystemService(DisplayManager.class).getDisplay(

                Display.DEFAULT_DISPLAY);

        when(mDisplayManager.getDisplay(anyInt())).thenReturn(mDisplay);



        mSupplier = new FakeSwitchSupplier(mDisplayManager);

        mModeSwitchesController = new ModeSwitchesController(mSupplier);

        mModeSwitchesController.setClickListenerDelegate(mListener);

        WindowManager wm = mContext.getSystemService(WindowManager.class);
@@ -107,6 +123,40 @@ public class ModeSwitchesControllerTest extends SysuiTestCase { 
        verify(mListener).onClick(mContext.getDisplayId());

    }



    @Test

    @DisableFlags(Flags.FLAG_CLEANUP_INSTANCES_WHEN_DISPLAY_REMOVED)

    public void testOnDisplayRemoved_flagOff_instancesStayInSupplier() {

        int originalCachedItemsSize = mSupplier.getSize();

        int testDisplayId2 = 200;

        int testDisplayId3 = 300;



        // Make the settings supplier add 2 new instance entries.

        mModeSwitchesController.removeButton(testDisplayId2);

        mModeSwitchesController.removeButton(testDisplayId3);

        // When displays removed, the current behavior keeps the entries/instances in the supplier.

        mModeSwitchesController.onDisplayRemoved(testDisplayId2);

        mModeSwitchesController.onDisplayRemoved(testDisplayId3);



        assertThat(mSupplier.getSize()).isEqualTo(originalCachedItemsSize + 2);

    }



    @Test

    @EnableFlags(Flags.FLAG_CLEANUP_INSTANCES_WHEN_DISPLAY_REMOVED)

    public void testOnDisplayRemoved_flagOn_instancesAreRemovedFromSupplier() {

        int originalCachedItemsSize = mSupplier.getSize();

        int testDisplayId2 = 200;

        int testDisplayId3 = 300;



        // Make the settings supplier add 2 new instance entries.

        mModeSwitchesController.removeButton(testDisplayId2);

        mModeSwitchesController.removeButton(testDisplayId3);

        // When displays removed, the related instance caches should be removed too.

        mModeSwitchesController.onDisplayRemoved(testDisplayId2);

        mModeSwitchesController.onDisplayRemoved(testDisplayId3);



        assertThat(mSupplier.getSize()).isEqualTo(originalCachedItemsSize);

    }



    private class FakeSwitchSupplier extends DisplayIdIndexSupplier<MagnificationModeSwitch> {



        FakeSwitchSupplier(DisplayManager displayManager) {

packages/SystemUI/src/com/android/systemui/accessibility/MagnificationImpl.java

+11 −0
Original line number Diff line number Diff line
@@ -48,6 +48,7 @@ import androidx.annotation.NonNull; 
import com.android.internal.accessibility.util.AccessibilityUtils;

import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.graphics.SfVsyncFrameCallbackProvider;

import com.android.systemui.Flags;

import com.android.systemui.LauncherProxyService;

import com.android.systemui.dagger.SysUISingleton;

import com.android.systemui.dagger.qualifiers.Main;
@@ -381,6 +382,16 @@ public class MagnificationImpl implements Magnification, CommandQueue.Callbacks 
        }

    }



    @Override

    @MainThread

    public void onDisplayRemoved(int displayId) {

        if (Flags.cleanupInstancesWhenDisplayRemoved()) {

            // Cleanup the cached instances for specific display

            mMagnificationSettingsSupplier.remove(displayId);

            mModeSwitchesController.onDisplayRemoved(displayId);

        }

    }



    @MainThread

    void updateSettingsButtonStatus(int displayId,

            @WindowMagnificationSettings.MagnificationSize int index) {

packages/SystemUI/src/com/android/systemui/accessibility/ModeSwitchesController.java

+14 −0
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import android.view.Display; 
import android.view.WindowManager;



import com.android.internal.annotations.VisibleForTesting;

import com.android.systemui.Flags;

import com.android.systemui.dagger.SysUISingleton;

import com.android.systemui.utils.windowmanager.WindowManagerProvider;
@@ -104,6 +105,19 @@ public class ModeSwitchesController implements ClickListener { 
                switchController -> switchController.onConfigurationChanged(configDiff));

    }



    /**

     * Called when the specific displayId is removed to cleanup.

     *

     * @param displayId The logical display id

     */

    @MainThread

    void onDisplayRemoved(int displayId) {

        if (Flags.cleanupInstancesWhenDisplayRemoved()) {

            removeButton(displayId);

            mSwitchSupplier.remove(displayId);

        }

    }



    @Override

    public void onClick(int displayId) {

        if (mClickListenerDelegate != null) {

packages/SystemUI/tests/src/com/android/systemui/accessibility/MagnificationTest.java

+55 −6
Original line number Diff line number Diff line
@@ -19,11 +19,13 @@ package com.android.systemui.accessibility; 
import static android.provider.Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN;

import static android.provider.Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_WINDOW;



import static com.android.systemui.LauncherProxyService.LauncherProxyListener;

import static com.android.systemui.accessibility.AccessibilityLogger.MagnificationSettingsEvent;

import static com.android.systemui.accessibility.WindowMagnificationSettings.MagnificationSize;

import static com.android.systemui.LauncherProxyService.LauncherProxyListener;

import static com.android.systemui.shared.system.QuickStepContract.SYSUI_STATE_MAGNIFICATION_OVERLAP;



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



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.ArgumentMatchers.anyInt;
@@ -41,6 +43,8 @@ import android.graphics.Rect; 
import android.hardware.display.DisplayManager;

import android.hardware.input.InputManager;

import android.os.RemoteException;

import android.platform.test.annotations.DisableFlags;

import android.platform.test.annotations.EnableFlags;

import android.testing.TestableLooper;

import android.view.Display;

import android.view.IWindowManager;
@@ -51,6 +55,7 @@ import android.view.accessibility.IMagnificationConnectionCallback; 
import androidx.test.ext.junit.runners.AndroidJUnit4;

import androidx.test.filters.SmallTest;



import com.android.systemui.Flags;

import com.android.systemui.LauncherProxyService;

import com.android.systemui.SysuiTestCase;

import com.android.systemui.model.SysUiState;
@@ -73,6 +78,8 @@ public class MagnificationTest extends SysuiTestCase { 


    private static final int TEST_DISPLAY = Display.DEFAULT_DISPLAY;

    @Mock

    private DisplayManager mDisplayManager;

    @Mock

    private AccessibilityManager mAccessibilityManager;

    @Mock

    private ModeSwitchesController mModeSwitchesController;
@@ -85,11 +92,14 @@ public class MagnificationTest extends SysuiTestCase { 
    @Mock

    private SecureSettings mSecureSettings;



    private Display mDisplay;

    private CommandQueue mCommandQueue;

    private MagnificationImpl mMagnification;

    private LauncherProxyListener mLauncherProxyListener;

    private FakeDisplayTracker mDisplayTracker = new FakeDisplayTracker(mContext);



    private FakeSettingsSupplier mSettingsSupplier;



    @Mock

    private WindowMagnificationController mWindowMagnificationController;

    @Mock
@@ -114,6 +124,10 @@ public class MagnificationTest extends SysuiTestCase { 
        }).when(mAccessibilityManager).setMagnificationConnection(

                any(IMagnificationConnection.class));



        mDisplay = mContext.getSystemService(DisplayManager.class).getDisplay(

                Display.DEFAULT_DISPLAY);

        when(mDisplayManager.getDisplay(anyInt())).thenReturn(mDisplay);



        when(mSysUiState.setFlag(anyLong(), anyBoolean())).thenReturn(mSysUiState);



        doAnswer(invocation -> {
@@ -134,13 +148,14 @@ public class MagnificationTest extends SysuiTestCase { 
                getContext().getMainThreadHandler(), mContext.getMainExecutor(),

                mCommandQueue, mModeSwitchesController,

                mSysUiState, mLauncherProxyService, mSecureSettings, mDisplayTracker,

                getContext().getSystemService(DisplayManager.class), mA11yLogger, mIWindowManager,

                mDisplayManager, mA11yLogger, mIWindowManager,

                getContext().getSystemService(AccessibilityManager.class), mWindowManagerProvider,

                mInputManager);

        mMagnification.mWindowMagnificationControllerSupplier = new FakeControllerSupplier(

                mContext.getSystemService(DisplayManager.class), mWindowMagnificationController);

        mMagnification.mMagnificationSettingsSupplier = new FakeSettingsSupplier(

                mContext.getSystemService(DisplayManager.class), mMagnificationSettingsController);

                mDisplayManager, mWindowMagnificationController);

        mSettingsSupplier = new FakeSettingsSupplier(

                mDisplayManager, mMagnificationSettingsController);

        mMagnification.mMagnificationSettingsSupplier = mSettingsSupplier;

        mMagnification.start();



        final ArgumentCaptor<LauncherProxyListener> listenerArgumentCaptor =
@@ -387,7 +402,7 @@ public class MagnificationTest extends SysuiTestCase { 
    public void overviewProxyIsConnected_controllerIsAvailable_updateSysUiStateFlag() {

        final WindowMagnificationController mController = mock(WindowMagnificationController.class);

        mMagnification.mWindowMagnificationControllerSupplier = new FakeControllerSupplier(

                mContext.getSystemService(DisplayManager.class), mController);

                mDisplayManager, mController);

        mMagnification.mWindowMagnificationControllerSupplier.get(TEST_DISPLAY);



        mLauncherProxyListener.onConnectionChanged(true);
@@ -395,6 +410,40 @@ public class MagnificationTest extends SysuiTestCase { 
        verify(mController).updateSysUIStateFlag();

    }



    @Test

    @DisableFlags(Flags.FLAG_CLEANUP_INSTANCES_WHEN_DISPLAY_REMOVED)

    public void onDisplayRemoved_flagOff_instancesStayInSupplier() {

        int originalCachedItemsSize = mSettingsSupplier.getSize();

        int testDisplayId2 = 200;

        int testDisplayId3 = 300;



        // Make the settings supplier add 2 new instance entries.

        mMagnification.hideMagnificationSettingsPanel(testDisplayId2);

        mMagnification.hideMagnificationSettingsPanel(testDisplayId3);

        // When displays removed, the current behavior keeps the entries/instances in the supplier.

        mDisplayTracker.triggerOnDisplayRemoved(testDisplayId2);

        mDisplayTracker.triggerOnDisplayRemoved(testDisplayId3);



        assertThat(mSettingsSupplier.getSize()).isEqualTo(originalCachedItemsSize + 2);

    }



    @Test

    @EnableFlags(Flags.FLAG_CLEANUP_INSTANCES_WHEN_DISPLAY_REMOVED)

    public void onDisplayRemoved_flagOn_instancesAreRemovedFromSupplier() {

        int originalCachedItemsSize = mSettingsSupplier.getSize();

        int testDisplayId2 = 200;

        int testDisplayId3 = 300;



        // Make the settings supplier add 2 new instance entries.

        mMagnification.hideMagnificationSettingsPanel(testDisplayId2);

        mMagnification.hideMagnificationSettingsPanel(testDisplayId3);

        // When displays removed, the related instance caches should be removed too.

        mDisplayTracker.triggerOnDisplayRemoved(testDisplayId2);

        mDisplayTracker.triggerOnDisplayRemoved(testDisplayId3);



        assertThat(mSettingsSupplier.getSize()).isEqualTo(originalCachedItemsSize);

    }



    private static class FakeControllerSupplier extends

            DisplayIdIndexSupplier<WindowMagnificationController> {