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

Commit a65450bf authored by Miranda Kephart's avatar Miranda Kephart
Browse files

Handle ScreenshotHelper references correctly 

The screenshot service is a singleton, but there may be multiple
instances of ScreenshotHelper, each with their own 'finish' callbacks
that tell them to reset their connection to the screenshot service. For
example, in general each invocation method has its own instance of
ScreenshotHelper. However, when we get a new invocation while the
ScreenshotController is running, we just update our pointer to the new
callback, losing the reference to the old callback.

This means that if screenshots are invoked with two different instances
of ScreenshotHelper simultaneously (e.g. by invoking via quick tap and
then by keychord while the UI is still up) we will never call finish()
on the first instance and it will keep holding on to its connection to
the screenshot service (so the screenshot service will never be
unbound).

This change keeps track of all the currently held callbacks and calls
them when the screenshot controller finishes.

Bug: 334551134
Fix: 334551134
Test: take two screenshots in succession using different invocation
methods; verify that the screenshot process is unbound and destroyed
Flag: EXEMPT minor change

Change-Id: I1df15abf82b5b1348485148956cd2f386aed5bdb
parent 20f915ec

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

+7 −5
Original line number Diff line number Diff line
@@ -97,12 +97,13 @@ internal constructor( 
    private val window: ScreenshotWindow

    private val actionExecutor: ActionExecutor

    private val copyBroadcastReceiver: BroadcastReceiver

    private val currentRequestCallbacks: MutableList<TakeScreenshotService.RequestCallback> =

        mutableListOf()



    private var screenshotSoundController: ScreenshotSoundController? = null

    private var screenBitmap: Bitmap? = null

    private var screenshotTakenInPortrait = false

    private var screenshotAnimation: Animator? = null

    private var currentRequestCallback: TakeScreenshotService.RequestCallback? = null

    private var packageName = ""



    /** Tracks config changes that require re-creating UI */
@@ -170,7 +171,6 @@ internal constructor( 
    ) {

        Assert.isMainThread()



        currentRequestCallback = requestCallback

        if (screenshot.type == TAKE_SCREENSHOT_FULLSCREEN && screenshot.bitmap == null) {

            val bounds = fullScreenRect

            screenshot.bitmap = imageCapture.captureDisplay(display.displayId, bounds)
@@ -181,7 +181,7 @@ internal constructor( 
        if (currentBitmap == null) {

            Log.e(TAG, "handleScreenshot: Screenshot bitmap was null")

            notificationController.notifyScreenshotError(R.string.screenshot_failed_to_capture_text)

            currentRequestCallback?.reportError()

            requestCallback.reportError()

            return

        }
@@ -194,8 +194,10 @@ internal constructor( 
            // User setup isn't complete, so we don't want to show any UI beyond a toast, as editing

            // and sharing shouldn't be exposed to the user.

            saveScreenshotAndToast(screenshot, finisher)

            requestCallback.onFinish()

            return

        }

        currentRequestCallbacks.add(requestCallback)



        broadcastSender.sendBroadcast(

            Intent(ClipboardOverlayController.SCREENSHOT_ACTION),
@@ -499,8 +501,8 @@ internal constructor( 
        Log.d(TAG, "finishDismiss")

        actionsController.endScreenshotSession()

        scrollCaptureExecutor.close()

        currentRequestCallback?.onFinish()

        currentRequestCallback = null

        currentRequestCallbacks.forEach { it.onFinish() }

        currentRequestCallbacks.clear()

        viewProxy.reset()

        removeWindow()

        screenshotHandler.cancelTimeout()