diff options
3 files changed, 37 insertions, 21 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt index 6c886fcd9c8f..439cedf12cbc 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt +++ b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotExecutor.kt @@ -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,25 +168,34 @@ 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()) { - Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_APP, TAG, id) - originalCallback.onFinish() - } + reportToOriginalIfNeeded() + } + + private fun endTrace(reason: String) { + Log.d(TAG, "Finished waiting for id=$id. $reason") + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_APP, TAG, id) + Trace.instantForTrack(Trace.TRACE_TAG_APP, TAG, reason) } } } + + private fun reportToOriginalIfNeeded() { + if (idsPending.isNotEmpty()) return + if (idsWithErrors.isEmpty()) { + originalCallback.onFinish() + } else { + originalCallback.reportError() + } + } } private companion object { diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java index 1e8542fe8f0c..32df5121b0b8 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/TakeScreenshotService.java @@ -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 */ diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt index 97c2ed45c26d..d945f50d4fec 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/TakeScreenshotExecutorTest.kt @@ -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() } |