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

Commit eb7f2968 authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato
Browse files

Prevent screenshot UI from being removed if only one screenshot fails 

This changes the logic to report screenshot errors to ScreenshotHelper.

Before this cl, if any display screenshot was failing for any reason, all the UI would have been removed, from all displays, as the TakeScreenshotService was sending a SCREENSHOT_MSG_PROCESS_COMPLETE to ScreenshotHelper that was resetting the connection.

Now, even if one display fails, the SCREENSHOT_MSG_PROCESS_COMPLETE is sent only after the UI of the other display is removed.

Note that this is unrelated with the notification of screenshot error.

Bug: 297355218
Bug: 290910794
Test: TakeScreenshotExecutorTest
Change-Id: I6e6d107048b46200dbff90167f5da37577e72421
parent 345fd7a6

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

+27 −16
Original line number Diff line number Diff line
@@ -144,16 +144,18 @@ constructor( 
    }



    /**

     * Returns a [RequestCallback] that calls [RequestCallback.onFinish] only when all callbacks for

     * id created have finished.

     * Returns a [RequestCallback] that wraps [originalCallback].

     *

     * If any callback created calls [reportError], then following [onFinish] are not considered.

     * Each [RequestCallback] created with [createCallbackForId] is expected to be used with either

     * [reportError] or [onFinish]. Once they are both called:

     * - If any finished with an error, [reportError] of [originalCallback] is called

     * - Otherwise, [onFinish] is called.

     */

    private class MultiResultCallbackWrapper(

        private val originalCallback: RequestCallback,

    ) {

        private val idsPending = mutableSetOf<Int>()

        private var errorReported = false

        private val idsWithErrors = mutableSetOf<Int>()



        /**

         * Creates a callback for [id].
@@ -166,23 +168,32 @@ constructor( 
            idsPending += id

            return object : RequestCallback {

                override fun reportError() {

                    Log.d(TAG, "ReportError id=$id")

                    Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_APP, TAG, id)

                    Trace.instantForTrack(Trace.TRACE_TAG_APP, TAG, "reportError id=$id")

                    originalCallback.reportError()

                    errorReported = true

                    endTrace("reportError id=$id")

                    idsWithErrors += id

                    idsPending -= id

                    reportToOriginalIfNeeded()

                }



                override fun onFinish() {

                    Log.d(TAG, "onFinish id=$id")

                    if (errorReported) return

                    endTrace("onFinish id=$id")

                    idsPending -= id

                    Trace.instantForTrack(Trace.TRACE_TAG_APP, TAG, "onFinish id=$id")

                    if (idsPending.isEmpty()) {

                    reportToOriginalIfNeeded()

                }



                private fun endTrace(reason: String) {

                    Log.d(TAG, "Finished waiting for id=$id. $reason")

                    Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_APP, TAG, id)

                        originalCallback.onFinish()

                    Trace.instantForTrack(Trace.TRACE_TAG_APP, TAG, reason)

                }

            }

        }



        private fun reportToOriginalIfNeeded() {

            if (idsPending.isNotEmpty()) return

            if (idsWithErrors.isEmpty()) {

                originalCallback.onFinish()

            } else {

                originalCallback.reportError()

            }

        }

    }

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

+5 −1
Original line number Diff line number Diff line
@@ -99,7 +99,11 @@ public class TakeScreenshotService extends Service { 


    /** Informs about coarse grained state of the Controller. */

    public interface RequestCallback {

        /** Respond to the current request indicating the screenshot request failed. */

        /**

         * Respond to the current request indicating the screenshot request failed.

         * <p>

         * After this, the service will be disconnected and all visible UI is removed.

         */

        void reportError();



        /** The controller has completed handling this request UI has been removed */

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

+5 −4
Original line number Diff line number Diff line
@@ -149,7 +149,7 @@ class TakeScreenshotExecutorTest : SysuiTestCase() { 
        }



    @Test

    fun executeScreenshots_doesNotReportFinishedIfOneFinishesOtherFails() =

    fun executeScreenshots_oneFinishesOtherFails_reportFailsOnlyAtTheEnd() =

        testScope.runTest {

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

            val onSaved = { _: Uri -> }
@@ -176,7 +176,7 @@ class TakeScreenshotExecutorTest : SysuiTestCase() { 
        }



    @Test

    fun executeScreenshots_doesNotReportFinishedAfterOneFails() =

    fun executeScreenshots_allDisplaysFail_reportsFail() =

        testScope.runTest {

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

            val onSaved = { _: Uri -> }
@@ -193,11 +193,12 @@ class TakeScreenshotExecutorTest : SysuiTestCase() { 
            capturer0.value.reportError()



            verify(callback, never()).onFinish()

            verify(callback).reportError()

            verify(callback, never()).reportError()



            capturer1.value.onFinish()

            capturer1.value.reportError()



            verify(callback, never()).onFinish()

            verify(callback).reportError()

            screenshotExecutor.onDestroy()

        }