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

Commit 40a2874f authored by Treehugger Robot's avatar Treehugger Robot Committed by Android (Google) Code Review
Browse files

Merge "fix(magnification): cleanup cached instance for removed display" into main

parents 2525179c a69ad6b7
Loading
Loading
Loading
Loading
+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"
+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) {
+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) {
+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) {
+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> {