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

Commit 5a3c7054 authored by Matt Casey's avatar Matt Casey Committed by Android (Google) Code Review
Browse files

Merge "Fix Race in ScreenshotController" into main

parents c6a68fea ec86f4b9

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

+22 −28
Original line number Diff line number Diff line
@@ -47,7 +47,6 @@ import android.content.res.Configuration; 
import android.graphics.Bitmap;

import android.graphics.Insets;

import android.graphics.Rect;

import android.hardware.display.DisplayManager;

import android.net.Uri;

import android.os.Process;

import android.os.UserHandle;
@@ -208,8 +207,7 @@ public class ScreenshotController { 
    @Nullable

    private final ScreenshotSoundController mScreenshotSoundController;

    private final PhoneWindow mWindow;

    private final DisplayManager mDisplayManager;

    private final int mDisplayId;

    private final Display mDisplay;

    private final ScrollCaptureExecutor mScrollCaptureExecutor;

    private final ScreenshotNotificationSmartActionsProvider

            mScreenshotNotificationSmartActionsProvider;
@@ -249,7 +247,6 @@ public class ScreenshotController { 
    @AssistedInject

    ScreenshotController(

            Context context,

            DisplayManager displayManager,

            WindowManager windowManager,

            FeatureFlags flags,

            ScreenshotViewProxy.Factory viewProxyFactory,
@@ -271,12 +268,13 @@ public class ScreenshotController { 
            AssistContentRequester assistContentRequester,

            MessageContainerController messageContainerController,

            Provider<ScreenshotSoundController> screenshotSoundController,

            @Assisted int displayId,

            @Assisted Display display,

            @Assisted boolean showUIOnExternalDisplay

    ) {

        mScreenshotSmartActions = screenshotSmartActions;

        mActionsProviderFactory = actionsProviderFactory;

        mNotificationsController = screenshotNotificationsControllerFactory.create(displayId);

        mNotificationsController = screenshotNotificationsControllerFactory.create(

                display.getDisplayId());

        mUiEventLogger = uiEventLogger;

        mImageExporter = imageExporter;

        mImageCapture = imageCapture;
@@ -290,11 +288,9 @@ public class ScreenshotController { 
        mScreenshotHandler = timeoutHandler;

        mScreenshotHandler.setDefaultTimeoutMillis(SCREENSHOT_CORNER_DEFAULT_TIMEOUT_MILLIS);





        mDisplayId = displayId;

        mDisplayManager = displayManager;

        mDisplay = display;

        mWindowManager = windowManager;

        final Context displayContext = context.createDisplayContext(getDisplay());

        final Context displayContext = context.createDisplayContext(display);

        mContext = (WindowContext) displayContext.createWindowContext(TYPE_SCREENSHOT, null);

        mFlags = flags;

        mActionIntentExecutor = actionIntentExecutor;
@@ -302,7 +298,7 @@ public class ScreenshotController { 
        mMessageContainerController = messageContainerController;

        mAssistContentRequester = assistContentRequester;



        mViewProxy = viewProxyFactory.getProxy(mContext, mDisplayId);

        mViewProxy = viewProxyFactory.getProxy(mContext, mDisplay.getDisplayId());



        mScreenshotHandler.setOnTimeoutRunnable(() -> {

            if (DEBUG_UI) {
@@ -328,7 +324,7 @@ public class ScreenshotController { 
                });



        // Sound is only reproduced from the controller of the default display.

        if (displayId == Display.DEFAULT_DISPLAY) {

        if (mDisplay.getDisplayId() == Display.DEFAULT_DISPLAY) {

            mScreenshotSoundController = screenshotSoundController.get();

        } else {

            mScreenshotSoundController = null;
@@ -356,7 +352,7 @@ public class ScreenshotController { 
        if (screenshot.getType() == WindowManager.TAKE_SCREENSHOT_FULLSCREEN

                && screenshot.getBitmap() == null) {

            Rect bounds = getFullScreenRect();

            screenshot.setBitmap(mImageCapture.captureDisplay(mDisplayId, bounds));

            screenshot.setBitmap(mImageCapture.captureDisplay(mDisplay.getDisplayId(), bounds));

            screenshot.setScreenBounds(bounds);

        }
@@ -459,7 +455,7 @@ public class ScreenshotController { 
    }



    private boolean shouldShowUi() {

        return mDisplayId == Display.DEFAULT_DISPLAY || mShowUIOnExternalDisplay;

        return mDisplay.getDisplayId() == Display.DEFAULT_DISPLAY || mShowUIOnExternalDisplay;

    }



    void prepareViewForNewScreenshot(@NonNull ScreenshotData screenshot, String oldPackageName) {
@@ -618,7 +614,7 @@ public class ScreenshotController { 


    private void requestScrollCapture(UserHandle owner) {

        mScrollCaptureExecutor.requestScrollCapture(

                mDisplayId,

                mDisplay.getDisplayId(),

                mWindow.getDecorView().getWindowToken(),

                (response) -> {

                    mUiEventLogger.log(ScreenshotEvent.SCREENSHOT_LONG_SCREENSHOT_IMPRESSION,
@@ -641,7 +637,8 @@ public class ScreenshotController { 
        }

        mUiEventLogger.log(ScreenshotEvent.SCREENSHOT_LONG_SCREENSHOT_REQUESTED, 0,

                response.getPackageName());

        Bitmap newScreenshot = mImageCapture.captureDisplay(mDisplayId, getFullScreenRect());

        Bitmap newScreenshot = mImageCapture.captureDisplay(mDisplay.getDisplayId(),

                getFullScreenRect());

        if (newScreenshot == null) {

            Log.e(TAG, "Failed to capture current screenshot for scroll transition!");

            return;
@@ -819,7 +816,8 @@ public class ScreenshotController { 
    private void saveScreenshotInBackground(

            ScreenshotData screenshot, UUID requestId, Consumer<Uri> finisher) {

        ListenableFuture<ImageExporter.Result> future = mImageExporter.export(mBgExecutor,

                requestId, screenshot.getBitmap(), screenshot.getUserOrDefault(), mDisplayId);

                requestId, screenshot.getBitmap(), screenshot.getUserOrDefault(),

                mDisplay.getDisplayId());

        future.addListener(() -> {

            try {

                ImageExporter.Result result = future.get();
@@ -861,7 +859,7 @@ public class ScreenshotController { 
        data.mActionsReadyListener = actionsReadyListener;

        data.mQuickShareActionsReadyListener = quickShareActionsReadyListener;

        data.owner = owner;

        data.displayId = mDisplayId;

        data.displayId = mDisplay.getDisplayId();



        if (mSaveInBgTask != null) {

            // just log success/failure for the pre-existing screenshot
@@ -986,13 +984,9 @@ public class ScreenshotController { 
        }

    }



    private Display getDisplay() {

        return mDisplayManager.getDisplay(mDisplayId);

    }



    private Rect getFullScreenRect() {

        DisplayMetrics displayMetrics = new DisplayMetrics();

        getDisplay().getRealMetrics(displayMetrics);

        mDisplay.getRealMetrics(displayMetrics);

        return new Rect(0, 0, displayMetrics.widthPixels, displayMetrics.heightPixels);

    }
@@ -1026,12 +1020,12 @@ public class ScreenshotController { 
    @AssistedFactory

    public interface Factory {

        /**

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

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

         *

         * @param displayId:               display to capture

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

         * @param display                 display to capture

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

         *                                display.

         */

        ScreenshotController create(int displayId, boolean showUIOnExternalDisplay);

        ScreenshotController create(Display display, boolean showUIOnExternalDisplay);

    }

}

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

+13 −10
Original line number Diff line number Diff line
@@ -68,11 +68,13 @@ constructor( 
        onSaved: (Uri?) -> Unit,

        requestCallback: RequestCallback

    ) {

        val displayIds = getDisplaysToScreenshot(screenshotRequest.type)

        val displays = getDisplaysToScreenshot(screenshotRequest.type)

        val resultCallbackWrapper = MultiResultCallbackWrapper(requestCallback)

        displayIds.forEach { displayId: Int ->

        displays.forEach { display ->

            val displayId = display.displayId

            Log.d(TAG, "Executing screenshot for display $displayId")

            dispatchToController(

                display = display,

                rawScreenshotData = ScreenshotData.fromRequest(screenshotRequest, displayId),

                onSaved =

                    if (displayId == Display.DEFAULT_DISPLAY) {
@@ -85,6 +87,7 @@ constructor( 


    /** All logging should be triggered only by this method. */

    private suspend fun dispatchToController(

        display: Display,

        rawScreenshotData: ScreenshotData,

        onSaved: (Uri?) -> Unit,

        callback: RequestCallback
@@ -104,8 +107,7 @@ constructor( 
        logScreenshotRequested(screenshotData)

        Log.d(TAG, "Screenshot request: $screenshotData")

        try {

            getScreenshotController(screenshotData.displayId)

                .handleScreenshot(screenshotData, onSaved, callback)

            getScreenshotController(display).handleScreenshot(screenshotData, onSaved, callback)

        } catch (e: IllegalStateException) {

            Log.e(TAG, "Error while ScreenshotController was handling ScreenshotData!", e)

            onFailedScreenshotRequest(screenshotData, callback)
@@ -135,12 +137,13 @@ constructor( 
        callback.reportError()

    }



    private suspend fun getDisplaysToScreenshot(requestType: Int): List<Int> {

    private suspend fun getDisplaysToScreenshot(requestType: Int): List<Display> {

        val allDisplays = displays.first()

        return if (requestType == TAKE_SCREENSHOT_PROVIDED_IMAGE) {

            // If this is a provided image, let's show the UI on the default display only.

            listOf(Display.DEFAULT_DISPLAY)

            allDisplays.filter { it.displayId == Display.DEFAULT_DISPLAY }

        } else {

            displays.first().filter { it.type in ALLOWED_DISPLAY_TYPES }.map { it.displayId }

            allDisplays.filter { it.type in ALLOWED_DISPLAY_TYPES }

        }

    }
@@ -170,9 +173,9 @@ constructor( 
        screenshotControllers.clear()

    }



    private fun getScreenshotController(id: Int): ScreenshotController {

        return screenshotControllers.computeIfAbsent(id) {

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

    private fun getScreenshotController(display: Display): ScreenshotController {

        return screenshotControllers.computeIfAbsent(display.displayId) {

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

        }

    }

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

+13 −8
Original line number Diff line number Diff line
@@ -69,8 +69,9 @@ class TakeScreenshotExecutorTest : SysuiTestCase() { 


    @Before

    fun setUp() {

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

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

        whenever(controllerFactory.create(any(), any())).thenAnswer {

            if (it.getArgument<Display>(0).displayId == 0) controller0 else controller1

        }

        whenever(notificationControllerFactory.create(eq(0))).thenReturn(notificationsController0)

        whenever(notificationControllerFactory.create(eq(1))).thenReturn(notificationsController1)

    }
@@ -78,12 +79,14 @@ class TakeScreenshotExecutorTest : SysuiTestCase() { 
    @Test

    fun executeScreenshots_severalDisplays_callsControllerForEachOne() =

        testScope.runTest {

            setDisplays(display(TYPE_INTERNAL, id = 0), display(TYPE_EXTERNAL, id = 1))

            val internalDisplay = display(TYPE_INTERNAL, id = 0)

            val externalDisplay = display(TYPE_EXTERNAL, id = 1)

            setDisplays(internalDisplay, externalDisplay)

            val onSaved = { _: Uri? -> }

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



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

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

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

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



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

    fun executeScreenshots_providedImageType_callsOnlyDefaultDisplayController() =

        testScope.runTest {

            setDisplays(display(TYPE_INTERNAL, id = 0), display(TYPE_EXTERNAL, id = 1))

            val internalDisplay = display(TYPE_INTERNAL, id = 0)

            val externalDisplay = display(TYPE_EXTERNAL, id = 1)

            setDisplays(internalDisplay, externalDisplay)

            val onSaved = { _: Uri? -> }

            screenshotExecutor.executeScreenshots(

                createScreenshotRequest(TAKE_SCREENSHOT_PROVIDED_IMAGE),
@@ -115,8 +120,8 @@ class TakeScreenshotExecutorTest : SysuiTestCase() { 
                callback

            )



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

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

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

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



            val capturer = ArgumentCaptor<ScreenshotData>()