Fix double call to ScreenshotController

When SCREENSHOT_REQUEST_PROCESSOR is enabled, the request is
dispatched via RequestProcessor callback. This should return
without following the original code path without the flag
enabled.

Added a test case to ensure the request is only forwarded once.

Bug: 159422805
Test: atest TakeScreenshotServiceTest
Change-Id: I8df2dd8e97e1aadde9bbeb141fe29940f9b6a5ba
This commit is contained in:
Mark Renouf
2022-08-29 14:21:28 -04:00
parent 7823627d7c
commit 8f5e795498
2 changed files with 47 additions and 8 deletions

View File

@@ -229,6 +229,7 @@ public class TakeScreenshotService extends Service {
Log.d(TAG, "handleMessage: Using request processor");
mProcessor.processAsync(request,
(r) -> dispatchToController(r, onSaved, callback));
return;
}
dispatchToController(request, onSaved, callback);

View File

@@ -40,6 +40,7 @@ import com.android.internal.util.ScreenshotHelper.ScreenshotRequest
import com.android.systemui.SysuiTestCase
import com.android.systemui.flags.FakeFeatureFlags
import com.android.systemui.flags.Flags.SCREENSHOT_REQUEST_PROCESSOR
import com.android.systemui.flags.Flags.SCREENSHOT_WORK_PROFILE_POLICY
import com.android.systemui.screenshot.ScreenshotEvent.SCREENSHOT_REQUESTED_KEY_CHORD
import com.android.systemui.screenshot.ScreenshotEvent.SCREENSHOT_REQUESTED_OVERVIEW
import com.android.systemui.screenshot.TakeScreenshotService.RequestCallback
@@ -47,12 +48,15 @@ import com.android.systemui.util.mockito.any
import com.android.systemui.util.mockito.argThat
import com.android.systemui.util.mockito.eq
import com.android.systemui.util.mockito.mock
import java.util.function.Consumer
import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentMatchers.anyInt
import org.mockito.ArgumentMatchers.isNull
import org.mockito.Mockito.doAnswer
import org.mockito.Mockito.times
import org.mockito.Mockito.verify
import org.mockito.Mockito.verifyZeroInteractions
import org.mockito.Mockito.`when` as whenever
@@ -88,7 +92,16 @@ class TakeScreenshotServiceTest : SysuiTestCase() {
.thenReturn(false)
whenever(userManager.isUserUnlocked).thenReturn(true)
// Stub request processor as a synchronous no-op for tests with the flag enabled
doAnswer {
val request: ScreenshotRequest = it.getArgument(0) as ScreenshotRequest
val consumer: Consumer<ScreenshotRequest> = it.getArgument(1)
consumer.accept(request)
}.`when`(requestProcessor).processAsync(/* request= */ any(), /* callback= */ any())
// Flipped in selected test cases
flags.set(SCREENSHOT_REQUEST_PROCESSOR, false)
flags.set(SCREENSHOT_WORK_PROFILE_POLICY, false)
service.attach(
mContext,
@@ -105,10 +118,10 @@ class TakeScreenshotServiceTest : SysuiTestCase() {
service.onBind(null /* unused: Intent */)
service.onUnbind(null /* unused: Intent */)
verify(controller).removeWindow()
verify(controller, times(1)).removeWindow()
service.onDestroy()
verify(controller).onDestroy()
verify(controller, times(1)).onDestroy()
}
@Test
@@ -120,7 +133,32 @@ class TakeScreenshotServiceTest : SysuiTestCase() {
service.handleRequest(request, { /* onSaved */ }, callback)
verify(controller).takeScreenshotFullscreen(
verify(controller, times(1)).takeScreenshotFullscreen(
eq(topComponent),
/* onSavedListener = */ any(),
/* requestCallback = */ any())
assertEquals("Expected one UiEvent", eventLogger.numLogs(), 1)
val logEvent = eventLogger.get(0)
assertEquals("Expected SCREENSHOT_REQUESTED UiEvent",
logEvent.eventId, SCREENSHOT_REQUESTED_KEY_CHORD.id)
assertEquals("Expected supplied package name",
topComponent.packageName, eventLogger.get(0).packageName)
}
@Test
fun takeScreenshot_requestProcessorEnabled() {
flags.set(SCREENSHOT_REQUEST_PROCESSOR, true)
val request = ScreenshotRequest(
TAKE_SCREENSHOT_FULLSCREEN,
SCREENSHOT_KEY_CHORD,
topComponent)
service.handleRequest(request, { /* onSaved */ }, callback)
verify(controller, times(1)).takeScreenshotFullscreen(
eq(topComponent),
/* onSavedListener = */ any(),
/* requestCallback = */ any())
@@ -143,7 +181,7 @@ class TakeScreenshotServiceTest : SysuiTestCase() {
service.handleRequest(request, { /* onSaved */ }, callback)
verify(controller).takeScreenshotPartial(
verify(controller, times(1)).takeScreenshotPartial(
/* topComponent = */ isNull(),
/* onSavedListener = */ any(),
/* requestCallback = */ any())
@@ -167,7 +205,7 @@ class TakeScreenshotServiceTest : SysuiTestCase() {
service.handleRequest(request, { /* onSaved */ }, callback)
verify(controller).handleImageAsScreenshot(
verify(controller, times(1)).handleImageAsScreenshot(
argThat { b -> b.equalsHardwareBitmap(bitmap) },
eq(bounds),
eq(Insets.NONE), eq(TASK_ID), eq(USER_ID), eq(topComponent),
@@ -193,8 +231,8 @@ class TakeScreenshotServiceTest : SysuiTestCase() {
service.handleRequest(request, { /* onSaved */ }, callback)
verify(notificationsController).notifyScreenshotError(anyInt())
verify(callback).reportError()
verify(notificationsController, times(1)).notifyScreenshotError(anyInt())
verify(callback, times(1)).reportError()
verifyZeroInteractions(controller)
}
@@ -217,7 +255,7 @@ class TakeScreenshotServiceTest : SysuiTestCase() {
service.handleRequest(request, { /* onSaved */ }, callback)
// error shown: Toast.makeText(...).show(), untestable
verify(callback).reportError()
verify(callback, times(1)).reportError()
verifyZeroInteractions(controller)
}
}