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

Commit 77956a62 authored by Matt Casey's avatar Matt Casey
Browse files

Remove duplicated screenshot capture. 

It's only being triggered in screenshot controller[s] if it fails in the
policy processor. That first failure should trigger the error case
directly (it's already attached to a runCatching block that shows an
error notification if needed).

Test: Force error, verify that notification appears.
Test: atest PolicyRequestProcessorTest
Flag: EXEMPT duplicated functionality removal
Bug: 371568662
Change-Id: I7954f78e7fbea1e96d388a4ca919926a87fe59e2
parent b28ca285

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

+0 −5
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ package com.android.systemui.screenshot 
import android.net.Uri

import android.os.UserManager

import android.util.Log

import android.view.WindowManager

import com.android.internal.logging.UiEventLogger

import com.android.systemui.dagger.qualifiers.Main

import com.android.systemui.res.R
@@ -51,10 +50,6 @@ constructor( 
        finisher: Consumer<Uri?>,

        requestCallback: TakeScreenshotService.RequestCallback,

    ) {

        if (screenshot.type == WindowManager.TAKE_SCREENSHOT_FULLSCREEN) {

            screenshot.bitmap = imageCapture.captureDisplay(screenshot.displayId, crop = null)

        }



        if (screenshot.bitmap == null) {

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

            notificationsControllerFactory

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

+1 −6
Original line number Diff line number Diff line
@@ -255,12 +255,6 @@ public class LegacyScreenshotController implements InteractiveScreenshotHandler 
        Assert.isMainThread();



        mCurrentRequestCallback = requestCallback;

        Rect bounds = screenshot.getOriginalScreenBounds();

        if (screenshot.getType() == WindowManager.TAKE_SCREENSHOT_FULLSCREEN

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

            bounds = getFullScreenRect();

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

        }



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

            Log.e(TAG, "handleScreenshot: Screenshot bitmap was null");
@@ -323,6 +317,7 @@ public class LegacyScreenshotController implements InteractiveScreenshotHandler 


        attachWindow();



        Rect bounds = screenshot.getOriginalScreenBounds();

        boolean showFlash;

        if (screenshot.getType() == WindowManager.TAKE_SCREENSHOT_PROVIDED_IMAGE) {

            if (bounds != null

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

+0 −6
Original line number Diff line number Diff line
@@ -35,7 +35,6 @@ import android.util.Log 
import android.view.Display

import android.view.ScrollCaptureResponse

import android.view.ViewRootImpl.ActivityConfigCallback

import android.view.WindowManager.TAKE_SCREENSHOT_FULLSCREEN

import android.view.WindowManager.TAKE_SCREENSHOT_PROVIDED_IMAGE

import android.widget.Toast

import android.window.WindowContext
@@ -162,11 +161,6 @@ internal constructor( 
        Assert.isMainThread()

        screenshotHandler.resetTimeout()



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

            val bounds = fullScreenRect

            screenshot.bitmap = imageCapture.captureDisplay(display.displayId, bounds)

        }



        val currentBitmap = screenshot.bitmap

        if (currentBitmap == null) {

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

packages/SystemUI/src/com/android/systemui/screenshot/policy/PolicyRequestProcessor.kt

+4 −4
Original line number Diff line number Diff line
@@ -126,7 +126,7 @@ class PolicyRequestProcessor( 
        )

    }



    suspend fun replaceWithTaskSnapshot(

    private suspend fun replaceWithTaskSnapshot(

        original: ScreenshotData,

        componentName: ComponentName?,

        owner: UserHandle,
@@ -134,7 +134,7 @@ class PolicyRequestProcessor( 
        taskBounds: Rect?,

    ): ScreenshotData {

        Log.i(TAG, "Capturing task snapshot: $componentName / $owner")

        val taskSnapshot = capture.captureTask(taskId)

        val taskSnapshot = capture.captureTask(taskId) ?: error("Failed to capture task")

        return original.copy(

            type = TAKE_SCREENSHOT_PROVIDED_IMAGE,

            bitmap = taskSnapshot,
@@ -153,13 +153,13 @@ class PolicyRequestProcessor( 
        taskId: Int? = null,

    ): ScreenshotData {

        Log.i(TAG, "Capturing screenshot: $componentName / $owner")

        val screenshot = captureDisplay(displayId)

        val screenshot = captureDisplay(displayId) ?: error("Failed to capture screenshot")

        return original.copy(

            type = TAKE_SCREENSHOT_FULLSCREEN,

            bitmap = screenshot,

            userHandle = owner,

            topComponent = componentName,

            originalScreenBounds = Rect(0, 0, screenshot?.width ?: 0, screenshot?.height ?: 0),

            originalScreenBounds = Rect(0, 0, screenshot.width, screenshot.height),

            taskId = taskId ?: -1,

        )

    }

packages/SystemUI/tests/src/com/android/systemui/screenshot/policy/PolicyRequestProcessorTest.kt

+68 −9
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 
package com.android.systemui.screenshot.policy



import android.content.ComponentName

import android.graphics.Bitmap

import android.graphics.Insets

import android.graphics.Rect

import android.os.UserHandle
@@ -27,10 +28,12 @@ import com.android.systemui.screenshot.ImageCapture 
import com.android.systemui.screenshot.ScreenshotData

import com.android.systemui.screenshot.data.model.DisplayContentScenarios.ActivityNames.FILES

import com.android.systemui.screenshot.data.model.DisplayContentScenarios.TaskSpec

import com.android.systemui.screenshot.data.model.DisplayContentScenarios.launcherOnly

import com.android.systemui.screenshot.data.model.DisplayContentScenarios.singleFullScreen

import com.android.systemui.screenshot.data.repository.DisplayContentRepository

import com.android.systemui.screenshot.policy.TestUserIds.PERSONAL

import com.android.systemui.screenshot.policy.TestUserIds.WORK

import com.google.common.truth.Truth.assertThat

import com.google.common.truth.Truth.assertWithMessage

import kotlinx.coroutines.Dispatchers

import kotlinx.coroutines.runBlocking
@@ -39,14 +42,6 @@ import org.junit.runner.RunWith 


@RunWith(AndroidJUnit4::class)

class PolicyRequestProcessorTest {



    val imageCapture =

        object : ImageCapture {

            override fun captureDisplay(displayId: Int, crop: Rect?) = null



            override suspend fun captureTask(taskId: Int) = null

        }



    /** Tests behavior when no policies are applied */

    @Test

    fun testProcess_defaultOwner_whenNoPolicyApplied() {
@@ -71,7 +66,7 @@ class PolicyRequestProcessorTest { 
        val requestProcessor =

            PolicyRequestProcessor(

                Dispatchers.Unconfined,

                imageCapture,

                createImageCapture(),

                policies = emptyList(),

                defaultOwner = UserHandle.of(PERSONAL),

                defaultComponent = ComponentName("default", "Component"),
@@ -91,6 +86,70 @@ class PolicyRequestProcessorTest { 
        assertWithMessage("Task ID").that(result.taskId).isEqualTo(TASK_ID)

    }



    @Test

    fun testProcess_throwsWhenCaptureFails() {

        val request = ScreenshotData.forTesting()



        /* Create a policy request processor with no capture policies */

        val requestProcessor =

            PolicyRequestProcessor(

                Dispatchers.Unconfined,

                createImageCapture(display = null),

                policies = emptyList(),

                defaultComponent = ComponentName("default", "Component"),

                displayTasks = DisplayContentRepository { launcherOnly() },

            )



        val result = runCatching { runBlocking { requestProcessor.process(request) } }



        assertThat(result.isFailure).isTrue()

    }



    @Test

    fun testProcess_throwsWhenTaskCaptureFails() {

        val request = ScreenshotData.forTesting()

        val fullScreenWork = DisplayContentRepository {

            singleFullScreen(TaskSpec(taskId = TASK_ID, name = FILES, userId = WORK))

        }



        val captureTaskPolicy = CapturePolicy {

            CapturePolicy.PolicyResult.Matched(

                policy = "",

                reason = "",

                parameters =

                    CaptureParameters(

                        CaptureType.IsolatedTask(taskId = 0, taskBounds = null),

                        null,

                        UserHandle.CURRENT,

                    ),

            )

        }



        /* Create a policy request processor with no capture policies */

        val requestProcessor =

            PolicyRequestProcessor(

                Dispatchers.Unconfined,

                createImageCapture(task = null),

                policies = listOf(captureTaskPolicy),

                defaultComponent = ComponentName("default", "Component"),

                displayTasks = fullScreenWork,

            )



        val result = runCatching { runBlocking { requestProcessor.process(request) } }



        assertThat(result.isFailure).isTrue()

    }



    private fun createImageCapture(

        display: Bitmap? = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888),

        task: Bitmap? = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888),

    ) =

        object : ImageCapture {

            override fun captureDisplay(displayId: Int, crop: Rect?) = display



            override suspend fun captureTask(taskId: Int) = task

        }



    companion object {

        const val TASK_ID = 1001

    }