Fix swapped int params in DisplayContentInfo ctor

Changes the userId parameter to a UserHandle to avoid this
particular hazard again. Adds test coverage for this code.

Bug: 159422805
Test: atest RequestProcessorTest ScreenshotPolicyImplTest
Change-Id: I8d65b4fb5835036cf0129403a806b739d2f9acdc

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

@@ -17,6 +17,7 @@
 package com.android.systemui.screenshot
 
 import android.graphics.Insets
+import android.util.Log
 import android.view.WindowManager.TAKE_SCREENSHOT_PROVIDED_IMAGE
 import com.android.internal.util.ScreenshotHelper.HardwareBitmapBundler
 import com.android.internal.util.ScreenshotHelper.ScreenshotRequest
@@ -61,8 +62,9 @@ class RequestProcessor @Inject constructor(
         ) {
 
             val info = policy.findPrimaryContent(policy.getDefaultDisplayId())
+            Log.d(TAG, "findPrimaryContent: $info")
 
-            result = if (policy.isManagedProfile(info.userId)) {
+            result = if (policy.isManagedProfile(info.user.identifier)) {
                 val image = capture.captureTask(info.taskId)
                     ?: error("Task snapshot returned a null Bitmap!")
 
@@ -70,7 +72,7 @@ class RequestProcessor @Inject constructor(
                 ScreenshotRequest(
                     TAKE_SCREENSHOT_PROVIDED_IMAGE, request.source,
                     HardwareBitmapBundler.hardwareBitmapToBundle(image),
-                    info.bounds, Insets.NONE, info.taskId, info.userId, info.component
+                    info.bounds, Insets.NONE, info.taskId, info.user.identifier, info.component
                 )
             } else {
                 // Create a new request of the same type which includes the top component

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

@@ -19,6 +19,7 @@ package com.android.systemui.screenshot
 import android.annotation.UserIdInt
 import android.content.ComponentName
 import android.graphics.Rect
+import android.os.UserHandle
 import android.view.Display
 
 /**
@@ -42,7 +43,7 @@ interface ScreenshotPolicy {
     data class DisplayContentInfo(
         val component: ComponentName,
         val bounds: Rect,
-        @UserIdInt val userId: Int,
+        val user: UserHandle,
         val taskId: Int,
     )
 

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

@@ -29,9 +29,11 @@ import android.content.Intent
 import android.graphics.Rect
 import android.os.Process
 import android.os.RemoteException
+import android.os.UserHandle
 import android.os.UserManager
 import android.util.Log
 import android.view.Display.DEFAULT_DISPLAY
+import com.android.internal.annotations.VisibleForTesting
 import com.android.internal.infra.ServiceConnector
 import com.android.systemui.SystemUIService
 import com.android.systemui.dagger.SysUISingleton
@@ -45,21 +47,13 @@ import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.withContext
 
 @SysUISingleton
-internal class ScreenshotPolicyImpl @Inject constructor(
+internal open class ScreenshotPolicyImpl @Inject constructor(
     context: Context,
     private val userMgr: UserManager,
     private val atmService: IActivityTaskManager,
     @Background val bgDispatcher: CoroutineDispatcher,
 ) : ScreenshotPolicy {
 
-    private val systemUiContent =
-        DisplayContentInfo(
-            ComponentName(context, SystemUIService::class.java),
-            Rect(),
-            ActivityTaskManager.INVALID_TASK_ID,
-            Process.myUserHandle().identifier,
-        )
-
     private val proxyConnector: ServiceConnector<IScreenshotProxy> =
         ServiceConnector.Impl(
             context,
@@ -78,6 +72,9 @@ internal class ScreenshotPolicyImpl @Inject constructor(
     }
 
     private fun nonPipVisibleTask(info: RootTaskInfo): Boolean {
+        if (DEBUG) {
+            debugLogRootTaskInfo(info)
+        }
         return info.windowingMode != WindowConfiguration.WINDOWING_MODE_PINNED &&
             info.isVisible &&
             info.isRunning &&
@@ -99,58 +96,46 @@ internal class ScreenshotPolicyImpl @Inject constructor(
         }
 
         val taskInfoList = getAllRootTaskInfosOnDisplay(displayId)
-        if (DEBUG) {
-            debugLogRootTaskInfos(taskInfoList)
-        }
 
         // If no visible task is located, then report SystemUI as the foreground content
         val target = taskInfoList.firstOrNull(::nonPipVisibleTask) ?: return systemUiContent
-
-        val topActivity: ComponentName = target.topActivity ?: error("should not be null")
-        val topChildTask = target.childTaskIds.size - 1
-        val childTaskId = target.childTaskIds[topChildTask]
-        val childTaskUserId = target.childTaskUserIds[topChildTask]
-        val childTaskBounds = target.childTaskBounds[topChildTask]
-
-        return DisplayContentInfo(topActivity, childTaskBounds, childTaskId, childTaskUserId)
+        return target.toDisplayContentInfo()
     }
 
-    private fun debugLogRootTaskInfos(taskInfoList: List<RootTaskInfo>) {
-        for (info in taskInfoList) {
-            Log.d(
-                TAG,
-                "[root task info] " +
-                    "taskId=${info.taskId} " +
-                    "parentTaskId=${info.parentTaskId} " +
-                    "position=${info.position} " +
-                    "positionInParent=${info.positionInParent} " +
-                    "isVisible=${info.isVisible()} " +
-                    "visible=${info.visible} " +
-                    "isFocused=${info.isFocused} " +
-                    "isSleeping=${info.isSleeping} " +
-                    "isRunning=${info.isRunning} " +
-                    "windowMode=${windowingModeToString(info.windowingMode)} " +
-                    "activityType=${activityTypeToString(info.activityType)} " +
-                    "topActivity=${info.topActivity} " +
-                    "topActivityInfo=${info.topActivityInfo} " +
-                    "numActivities=${info.numActivities} " +
-                    "childTaskIds=${Arrays.toString(info.childTaskIds)} " +
-                    "childUserIds=${Arrays.toString(info.childTaskUserIds)} " +
-                    "childTaskBounds=${Arrays.toString(info.childTaskBounds)} " +
-                    "childTaskNames=${Arrays.toString(info.childTaskNames)}"
-            )
+    private fun debugLogRootTaskInfo(info: RootTaskInfo) {
+        Log.d(TAG, "RootTaskInfo={" +
+                "taskId=${info.taskId} " +
+                "parentTaskId=${info.parentTaskId} " +
+                "position=${info.position} " +
+                "positionInParent=${info.positionInParent} " +
+                "isVisible=${info.isVisible()} " +
+                "visible=${info.visible} " +
+                "isFocused=${info.isFocused} " +
+                "isSleeping=${info.isSleeping} " +
+                "isRunning=${info.isRunning} " +
+                "windowMode=${windowingModeToString(info.windowingMode)} " +
+                "activityType=${activityTypeToString(info.activityType)} " +
+                "topActivity=${info.topActivity} " +
+                "topActivityInfo=${info.topActivityInfo} " +
+                "numActivities=${info.numActivities} " +
+                "childTaskIds=${Arrays.toString(info.childTaskIds)} " +
+                "childUserIds=${Arrays.toString(info.childTaskUserIds)} " +
+                "childTaskBounds=${Arrays.toString(info.childTaskBounds)} " +
+                "childTaskNames=${Arrays.toString(info.childTaskNames)}" +
+                "}"
+        )
 
-            for (j in 0 until info.childTaskIds.size) {
-                Log.d(TAG, "    *** [$j] ******")
-                Log.d(TAG, "        ***  childTaskIds[$j]: ${info.childTaskIds[j]}")
-                Log.d(TAG, "        ***  childTaskUserIds[$j]: ${info.childTaskUserIds[j]}")
-                Log.d(TAG, "        ***  childTaskBounds[$j]: ${info.childTaskBounds[j]}")
-                Log.d(TAG, "        ***  childTaskNames[$j]: ${info.childTaskNames[j]}")
-            }
+        for (j in 0 until info.childTaskIds.size) {
+            Log.d(TAG, "    *** [$j] ******")
+            Log.d(TAG, "        ***  childTaskIds[$j]: ${info.childTaskIds[j]}")
+            Log.d(TAG, "        ***  childTaskUserIds[$j]: ${info.childTaskUserIds[j]}")
+            Log.d(TAG, "        ***  childTaskBounds[$j]: ${info.childTaskBounds[j]}")
+            Log.d(TAG, "        ***  childTaskNames[$j]: ${info.childTaskNames[j]}")
         }
     }
 
-    private suspend fun getAllRootTaskInfosOnDisplay(displayId: Int): List<RootTaskInfo> =
+    @VisibleForTesting
+    open suspend fun getAllRootTaskInfosOnDisplay(displayId: Int): List<RootTaskInfo> =
         withContext(bgDispatcher) {
             try {
                 atmService.getAllRootTaskInfosOnDisplay(displayId)
@@ -160,7 +145,8 @@ internal class ScreenshotPolicyImpl @Inject constructor(
             }
         }
 
-    private suspend fun isNotificationShadeExpanded(): Boolean = suspendCoroutine { k ->
+    @VisibleForTesting
+    open suspend fun isNotificationShadeExpanded(): Boolean = suspendCoroutine { k ->
         proxyConnector
             .postForResult { it.isNotificationShadeExpanded }
             .whenComplete { expanded, error ->
@@ -171,8 +157,30 @@ internal class ScreenshotPolicyImpl @Inject constructor(
             }
     }
 
-    companion object {
-        const val TAG: String = "ScreenshotPolicyImpl"
-        const val DEBUG: Boolean = false
-    }
+    @VisibleForTesting
+    internal val systemUiContent =
+        DisplayContentInfo(
+            ComponentName(context, SystemUIService::class.java),
+            Rect(),
+            Process.myUserHandle(),
+            ActivityTaskManager.INVALID_TASK_ID
+        )
+}
+
+private const val TAG: String = "ScreenshotPolicyImpl"
+private const val DEBUG: Boolean = false
+
+@VisibleForTesting
+internal fun RootTaskInfo.toDisplayContentInfo(): DisplayContentInfo {
+    val topActivity: ComponentName = topActivity ?: error("should not be null")
+    val topChildTask = childTaskIds.size - 1
+    val childTaskId = childTaskIds[topChildTask]
+    val childTaskUserId = childTaskUserIds[topChildTask]
+    val childTaskBounds = childTaskBounds[topChildTask]
+
+    return DisplayContentInfo(
+        topActivity,
+        childTaskBounds,
+        UserHandle.of(childTaskUserId),
+        childTaskId)
 }

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

@@ -23,6 +23,7 @@ import android.graphics.Insets
 import android.graphics.Rect
 import android.hardware.HardwareBuffer
 import android.os.Bundle
+import android.os.UserHandle
 import android.view.WindowManager.ScreenshotSource.SCREENSHOT_KEY_CHORD
 import android.view.WindowManager.ScreenshotSource.SCREENSHOT_OTHER
 import android.view.WindowManager.TAKE_SCREENSHOT_FULLSCREEN
@@ -97,7 +98,7 @@ class RequestProcessorTest {
         policy.setManagedProfile(USER_ID, false)
         policy.setDisplayContentInfo(
             policy.getDefaultDisplayId(),
-            DisplayContentInfo(component, bounds, USER_ID, TASK_ID))
+            DisplayContentInfo(component, bounds, UserHandle.of(USER_ID), TASK_ID))
 
         val request = ScreenshotRequest(TAKE_SCREENSHOT_FULLSCREEN, SCREENSHOT_KEY_CHORD)
         val processor = RequestProcessor(imageCapture, policy, flags, scope)
@@ -120,7 +121,7 @@ class RequestProcessorTest {
         // Indicate that the primary content belongs to a manged profile
         policy.setManagedProfile(USER_ID, true)
         policy.setDisplayContentInfo(policy.getDefaultDisplayId(),
-            DisplayContentInfo(component, bounds, USER_ID, TASK_ID))
+            DisplayContentInfo(component, bounds, UserHandle.of(USER_ID), TASK_ID))
 
         val request = ScreenshotRequest(TAKE_SCREENSHOT_FULLSCREEN, SCREENSHOT_KEY_CHORD)
         val processor = RequestProcessor(imageCapture, policy, flags, scope)
@@ -160,7 +161,7 @@ class RequestProcessorTest {
 
         policy.setManagedProfile(USER_ID, false)
         policy.setDisplayContentInfo(policy.getDefaultDisplayId(),
-            DisplayContentInfo(component, bounds, USER_ID, TASK_ID))
+            DisplayContentInfo(component, bounds, UserHandle.of(USER_ID), TASK_ID))
 
         val processedRequest = processor.process(request)
 
@@ -183,7 +184,7 @@ class RequestProcessorTest {
         // Indicate that the primary content belongs to a manged profile
         policy.setManagedProfile(USER_ID, true)
         policy.setDisplayContentInfo(policy.getDefaultDisplayId(),
-            DisplayContentInfo(component, bounds, USER_ID, TASK_ID))
+            DisplayContentInfo(component, bounds, UserHandle.of(USER_ID), TASK_ID))
 
         val processedRequest = processor.process(request)
 

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

@@ -0,0 +1,227 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.screenshot
+
+import android.app.ActivityTaskManager.RootTaskInfo
+import android.app.IActivityTaskManager
+import android.app.WindowConfiguration.ACTIVITY_TYPE_HOME
+import android.app.WindowConfiguration.ACTIVITY_TYPE_STANDARD
+import android.app.WindowConfiguration.ACTIVITY_TYPE_UNDEFINED
+import android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN
+import android.app.WindowConfiguration.WINDOWING_MODE_PINNED
+import android.content.ComponentName
+import android.content.Context
+import android.graphics.Rect
+import android.os.UserHandle
+import android.os.UserManager
+import android.testing.AndroidTestingRunner
+import com.android.systemui.SysuiTestCase
+import com.android.systemui.screenshot.ScreenshotPolicy.DisplayContentInfo
+import com.android.systemui.util.mockito.mock
+import com.google.common.truth.Truth.assertThat
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.runBlocking
+import org.junit.Test
+import org.junit.runner.RunWith
+
+// The following values are chosen to be distinct from commonly seen real values
+private const val DISPLAY_ID = 100
+private const val PRIMARY_USER = 2000
+private const val MANAGED_PROFILE_USER = 3000
+
+@RunWith(AndroidTestingRunner::class)
+class ScreenshotPolicyImplTest : SysuiTestCase() {
+
+    @Test
+    fun testToDisplayContentInfo() {
+        assertThat(fullScreenWorkProfileTask.toDisplayContentInfo())
+            .isEqualTo(
+                DisplayContentInfo(
+                    ComponentName(
+                        "com.google.android.apps.nbu.files",
+                        "com.google.android.apps.nbu.files.home.HomeActivity"
+                    ),
+                    Rect(0, 0, 1080, 2400),
+                    UserHandle.of(MANAGED_PROFILE_USER),
+                    65))
+    }
+
+    @Test
+    fun findPrimaryContent_ignoresPipTask() = runBlocking {
+        val policy = fakeTasksPolicyImpl(
+            mContext,
+            shadeExpanded = false,
+            tasks = listOf(
+                    pipTask,
+                    fullScreenWorkProfileTask,
+                    launcherTask,
+                    emptyTask)
+        )
+
+        val info = policy.findPrimaryContent(DISPLAY_ID)
+        assertThat(info).isEqualTo(fullScreenWorkProfileTask.toDisplayContentInfo())
+    }
+
+    @Test
+    fun findPrimaryContent_shadeExpanded_ignoresTopTask() = runBlocking {
+        val policy = fakeTasksPolicyImpl(
+            mContext,
+            shadeExpanded = true,
+            tasks = listOf(
+                fullScreenWorkProfileTask,
+                launcherTask,
+                emptyTask)
+        )
+
+        val info = policy.findPrimaryContent(DISPLAY_ID)
+        assertThat(info).isEqualTo(policy.systemUiContent)
+    }
+
+    @Test
+    fun findPrimaryContent_emptyTaskList() = runBlocking {
+        val policy = fakeTasksPolicyImpl(
+            mContext,
+            shadeExpanded = false,
+            tasks = listOf()
+        )
+
+        val info = policy.findPrimaryContent(DISPLAY_ID)
+        assertThat(info).isEqualTo(policy.systemUiContent)
+    }
+
+    @Test
+    fun findPrimaryContent_workProfileNotOnTop() = runBlocking {
+        val policy = fakeTasksPolicyImpl(
+            mContext,
+            shadeExpanded = false,
+            tasks = listOf(
+                launcherTask,
+                fullScreenWorkProfileTask,
+                emptyTask)
+        )
+
+        val info = policy.findPrimaryContent(DISPLAY_ID)
+        assertThat(info).isEqualTo(launcherTask.toDisplayContentInfo())
+    }
+
+    private fun fakeTasksPolicyImpl(
+        context: Context,
+        shadeExpanded: Boolean,
+        tasks: List<RootTaskInfo>
+    ): ScreenshotPolicyImpl {
+        val userManager = mock<UserManager>()
+        val atmService = mock<IActivityTaskManager>()
+        val dispatcher = Dispatchers.Unconfined
+
+        return object : ScreenshotPolicyImpl(context, userManager, atmService, dispatcher) {
+            override suspend fun isManagedProfile(userId: Int) = (userId == MANAGED_PROFILE_USER)
+            override suspend fun getAllRootTaskInfosOnDisplay(displayId: Int) = tasks
+            override suspend fun isNotificationShadeExpanded() = shadeExpanded
+        }
+    }
+
+    private val pipTask = RootTaskInfo().apply {
+        configuration.windowConfiguration.apply {
+            windowingMode = WINDOWING_MODE_PINNED
+            bounds = Rect(628, 1885, 1038, 2295)
+            activityType = ACTIVITY_TYPE_STANDARD
+        }
+        displayId = DISPLAY_ID
+        userId = PRIMARY_USER
+        taskId = 66
+        visible = true
+        isVisible = true
+        isRunning = true
+        numActivities = 1
+        topActivity = ComponentName(
+            "com.google.android.youtube",
+            "com.google.android.apps.youtube.app.watchwhile.WatchWhileActivity"
+        )
+        childTaskIds = intArrayOf(66)
+        childTaskNames = arrayOf("com.google.android.youtube/" +
+                "com.google.android.youtube.app.honeycomb.Shell\$HomeActivity")
+        childTaskUserIds = intArrayOf(0)
+        childTaskBounds = arrayOf(Rect(628, 1885, 1038, 2295))
+    }
+
+    private val fullScreenWorkProfileTask = RootTaskInfo().apply {
+        configuration.windowConfiguration.apply {
+            windowingMode = WINDOWING_MODE_FULLSCREEN
+            bounds = Rect(0, 0, 1080, 2400)
+            activityType = ACTIVITY_TYPE_STANDARD
+        }
+        displayId = DISPLAY_ID
+        userId = MANAGED_PROFILE_USER
+        taskId = 65
+        visible = true
+        isVisible = true
+        isRunning = true
+        numActivities = 1
+        topActivity = ComponentName(
+            "com.google.android.apps.nbu.files",
+            "com.google.android.apps.nbu.files.home.HomeActivity"
+        )
+        childTaskIds = intArrayOf(65)
+        childTaskNames = arrayOf("com.google.android.apps.nbu.files/" +
+                "com.google.android.apps.nbu.files.home.HomeActivity")
+        childTaskUserIds = intArrayOf(MANAGED_PROFILE_USER)
+        childTaskBounds = arrayOf(Rect(0, 0, 1080, 2400))
+    }
+
+    private val launcherTask = RootTaskInfo().apply {
+        configuration.windowConfiguration.apply {
+            windowingMode = WINDOWING_MODE_FULLSCREEN
+            bounds = Rect(0, 0, 1080, 2400)
+            activityType = ACTIVITY_TYPE_HOME
+        }
+        displayId = DISPLAY_ID
+        taskId = 1
+        userId = PRIMARY_USER
+        visible = true
+        isVisible = true
+        isRunning = true
+        numActivities = 1
+        topActivity = ComponentName(
+            "com.google.android.apps.nexuslauncher",
+            "com.google.android.apps.nexuslauncher.NexusLauncherActivity",
+        )
+        childTaskIds = intArrayOf(1)
+        childTaskNames = arrayOf("com.google.android.apps.nexuslauncher/" +
+                "com.google.android.apps.nexuslauncher.NexusLauncherActivity")
+        childTaskUserIds = intArrayOf(0)
+        childTaskBounds = arrayOf(Rect(0, 0, 1080, 2400))
+    }
+
+    private val emptyTask = RootTaskInfo().apply {
+        configuration.windowConfiguration.apply {
+            windowingMode = WINDOWING_MODE_FULLSCREEN
+            bounds = Rect(0, 0, 1080, 2400)
+            activityType = ACTIVITY_TYPE_UNDEFINED
+        }
+        displayId = DISPLAY_ID
+        taskId = 2
+        userId = PRIMARY_USER
+        visible = false
+        isVisible = false
+        isRunning = false
+        numActivities = 0
+        childTaskIds = intArrayOf(3, 4)
+        childTaskNames = arrayOf("", "")
+        childTaskUserIds = intArrayOf(0, 0)
+        childTaskBounds = arrayOf(Rect(0, 0, 1080, 2400), Rect(0, 2400, 1080, 4800))
+    }
+}