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

Commit 10ce9c0b authored by Nicolò Mazzucato's avatar Nicolò Mazzucato
Browse files

Avoid showing the screenshot ui on external displays 

When adding a window (the screenshot one in this case) to an external display that is mirroring the default one, mirroring stops.

For now we can't distinguish whether an external display is mirroring or extending the default one, so we're always passing "false" to "showUIOnExternalDisplay" ScreenshotController constructor to only show the UI on the default display

While a simple "showUi" would have worked from ScreenshotController point of view, the more verbose "showUIOnExternalDisplay" makes it  explicit that we can't avoid showing the UI on the default display.

Writing a test for ScreenshotController is not currently feasible due to the many deps that can't be mocked. Making it testable would require a full risky refactor.

Note that the screenshot window is still created regardless of the value of showUIOnExternalDisplay parameter: as a follow up improvement we can avoid the creation of it, but it would involve a big refactor of the class, not easy to flag-guard.

Bug: 290910794
Bug: 304693302
Test: Tested manually on felix with simulated external display
Change-Id: Id709a7c0ff5a3b3cda665ce44c7fa25727552a69
parent f9a6b1a7

packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java

+39 −13
Original line number Diff line number Diff line
@@ -303,6 +303,13 @@ public class ScreenshotController { 
    private String mPackageName = "";

    private BroadcastReceiver mCopyBroadcastReceiver;



    // When false, the screenshot is taken without showing the ui. Note that this only applies to

    // external displays, as on the default one the UI should **always** be shown.

    // This is needed in case of screenshot during display mirroring, as adding another window to

    // the external display makes mirroring stop.

    // When there is a way to distinguish between displays that are mirroring or extending, this

    // can be removed and we can directly show the ui only in the extended case.

    private final Boolean mShowUIOnExternalDisplay;

    /** Tracks config changes that require re-creating UI */

    private final InterestingConfigChanges mConfigChanges = new InterestingConfigChanges(

            ActivityInfo.CONFIG_ORIENTATION
@@ -335,7 +342,8 @@ public class ScreenshotController { 
            AssistContentRequester assistContentRequester,

            MessageContainerController messageContainerController,

            Provider<ScreenshotSoundController> screenshotSoundController,

            @Assisted int displayId

            @Assisted int displayId,

            @Assisted boolean showUIOnExternalDisplay

    ) {

        mScreenshotSmartActions = screenshotSmartActions;

        mNotificationsController = screenshotNotificationsController;
@@ -401,6 +409,7 @@ public class ScreenshotController { 
        mContext.registerReceiver(mCopyBroadcastReceiver, new IntentFilter(

                        ClipboardOverlayController.COPY_OVERLAY_ACTION),

                ClipboardOverlayController.SELF_PERMISSION, null, Context.RECEIVER_NOT_EXPORTED);

        mShowUIOnExternalDisplay = showUIOnExternalDisplay;

    }



    void handleScreenshot(ScreenshotData screenshot, Consumer<Uri> finisher,
@@ -448,6 +457,23 @@ public class ScreenshotController { 


        prepareViewForNewScreenshot(screenshot, oldPackageName);



        if (mFlags.isEnabled(Flags.SCREENSHOT_METADATA) && screenshot.getTaskId() >= 0) {

            mAssistContentRequester.requestAssistContent(screenshot.getTaskId(),

                    new AssistContentRequester.Callback() {

                        @Override

                        public void onAssistContentAvailable(AssistContent assistContent) {

                            screenshot.setContextUrl(assistContent.getWebUri());

                        }

                    });

        }



        if (!shouldShowUi()) {

            saveScreenshotInWorkerThread(

                screenshot.getUserHandle(), finisher, this::logSuccessOnActionsReady,

                (ignored) -> {});

            return;

        }



        saveScreenshotInWorkerThread(screenshot.getUserHandle(), finisher,

                this::showUiOnActionsReady, this::showUiOnQuickShareActionReady);
@@ -482,22 +508,16 @@ public class ScreenshotController { 
                screenshot.getUserHandle()));

        mScreenshotView.setScreenshot(screenshot);



        if (mFlags.isEnabled(Flags.SCREENSHOT_METADATA) && screenshot.getTaskId() >= 0) {

            mAssistContentRequester.requestAssistContent(screenshot.getTaskId(),

                    new AssistContentRequester.Callback() {

                        @Override

                        public void onAssistContentAvailable(AssistContent assistContent) {

                            screenshot.setContextUrl(assistContent.getWebUri());

                        }

                    });

        }



        // ignore system bar insets for the purpose of window layout

        mWindow.getDecorView().setOnApplyWindowInsetsListener(

                (v, insets) -> WindowInsets.CONSUMED);

        mScreenshotHandler.cancelTimeout(); // restarted after animation

    }



    private boolean shouldShowUi() {

        return mDisplayId == Display.DEFAULT_DISPLAY || mShowUIOnExternalDisplay;

    }



    void prepareViewForNewScreenshot(ScreenshotData screenshot, String oldPackageName) {

        withWindowAttached(() -> {

            if (mUserManager.isManagedProfile(screenshot.getUserHandle().getIdentifier())) {
@@ -1199,7 +1219,13 @@ public class ScreenshotController { 
    /** Injectable factory to create screenshot controller instances for a specific display. */

    @AssistedFactory

    public interface Factory {

        /** Creates an instance of the controller for that specific displayId. */

        ScreenshotController create(int displayId);

        /**

         * Creates an instance of the controller for that specific displayId.

         *

         * @param displayId:               display to capture

         * @param showUIOnExternalDisplay: Whether the UI should be shown if this is an external

         *                                 display.

         */

        ScreenshotController create(int displayId, boolean showUIOnExternalDisplay);

    }

}

packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt

+3 −1
Original line number Diff line number Diff line
@@ -135,7 +135,9 @@ constructor( 
    }



    private fun getScreenshotController(id: Int): ScreenshotController {

        return screenshotControllers.computeIfAbsent(id) { screenshotControllerFactory.create(id) }

        return screenshotControllers.computeIfAbsent(id) {

            screenshotControllerFactory.create(id, /* showUIOnExternalDisplay= */ false)

        }

    }



    /** For java compatibility only. see [executeScreenshots] */

packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java

+2 −1
Original line number Diff line number Diff line
@@ -132,7 +132,8 @@ public class TakeScreenshotService extends Service { 
        if (mFeatureFlags.isEnabled(MULTI_DISPLAY_SCREENSHOT)) {

            mScreenshot = null;

        } else {

            mScreenshot = screenshotControllerFactory.create(Display.DEFAULT_DISPLAY);

            mScreenshot = screenshotControllerFactory.create(

                    Display.DEFAULT_DISPLAY, /* showUIOnExternalDisplay= */ false);

        }

    }

packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt

+7 −7
Original line number Diff line number Diff line
@@ -63,8 +63,8 @@ class TakeScreenshotExecutorTest : SysuiTestCase() { 


    @Before

    fun setUp() {

        whenever(controllerFactory.create(eq(0))).thenReturn(controller0)

        whenever(controllerFactory.create(eq(1))).thenReturn(controller1)

        whenever(controllerFactory.create(eq(0), any())).thenReturn(controller0)

        whenever(controllerFactory.create(eq(1), any())).thenReturn(controller1)

    }



    @Test
@@ -74,8 +74,8 @@ class TakeScreenshotExecutorTest : SysuiTestCase() { 
            val onSaved = { _: Uri -> }

            screenshotExecutor.executeScreenshots(createScreenshotRequest(), onSaved, callback)



            verify(controllerFactory).create(eq(0))

            verify(controllerFactory).create(eq(1))

            verify(controllerFactory).create(eq(0), any())

            verify(controllerFactory).create(eq(1), any())



            val capturer = ArgumentCaptor<ScreenshotData>()
@@ -107,8 +107,8 @@ class TakeScreenshotExecutorTest : SysuiTestCase() { 
                callback

            )



            verify(controllerFactory).create(eq(0))

            verify(controllerFactory, never()).create(eq(1))

            verify(controllerFactory).create(eq(0), any())

            verify(controllerFactory, never()).create(eq(1), any())



            val capturer = ArgumentCaptor<ScreenshotData>()
@@ -139,7 +139,7 @@ class TakeScreenshotExecutorTest : SysuiTestCase() { 
    @Test

    fun executeScreenshots_allowedTypes_allCaptured() =

        testScope.runTest {

            whenever(controllerFactory.create(any())).thenReturn(controller0)

            whenever(controllerFactory.create(any(), any())).thenReturn(controller0)



            setDisplays(

                display(TYPE_INTERNAL, id = 0),

packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotServiceTest.kt

+1 −1
Original line number Diff line number Diff line
@@ -86,7 +86,7 @@ class TakeScreenshotServiceTest : SysuiTestCase() { 
            )

            .thenReturn(false)

        whenever(userManager.isUserUnlocked).thenReturn(true)

        whenever(controllerFactory.create(any())).thenReturn(controller)

        whenever(controllerFactory.create(any(), any())).thenReturn(controller)



        // Stub request processor as a synchronous no-op for tests with the flag enabled

        doAnswer {