Merge "Drop task snapshot with invalid hw buffer" into tm-dev

core/java/android/app/TaskStackListener.java

@@ -19,7 +19,6 @@ package android.app;
 import android.app.ActivityManager.RunningTaskInfo;
 import android.compat.annotation.UnsupportedAppUsage;
 import android.content.ComponentName;
-import android.os.Binder;
 import android.os.Build;
 import android.os.RemoteException;
 import android.window.TaskSnapshot;
@@ -32,10 +31,18 @@ import android.window.TaskSnapshot;
  */
 public abstract class TaskStackListener extends ITaskStackListener.Stub {
 
+    /** Whether this listener and the callback dispatcher are in different processes. */
+    private boolean mIsRemote = true;
+
     @UnsupportedAppUsage
     public TaskStackListener() {
     }
 
+    /** Indicates that this listener lives in system server. */
+    public void setIsLocal() {
+        mIsRemote = false;
+    }
+
     @Override
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
     public void onTaskStackChanged() throws RemoteException {
@@ -154,8 +161,7 @@ public abstract class TaskStackListener extends ITaskStackListener.Stub {
     @Override
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
     public void onTaskSnapshotChanged(int taskId, TaskSnapshot snapshot) throws RemoteException {
-        if (Binder.getCallingPid() != android.os.Process.myPid()
-                && snapshot != null && snapshot.getHardwareBuffer() != null) {
+        if (mIsRemote && snapshot != null && snapshot.getHardwareBuffer() != null) {
             // Preemptively clear any reference to the buffer
             snapshot.getHardwareBuffer().close();
         }

services/core/java/com/android/server/wm/TaskChangeNotificationController.java

@@ -20,6 +20,7 @@ import android.app.ActivityManager;
 import android.app.ActivityManager.RunningTaskInfo;
 import android.app.ITaskStackListener;
 import android.app.TaskInfo;
+import android.app.TaskStackListener;
 import android.content.ComponentName;
 import android.os.Binder;
 import android.os.Handler;
@@ -286,6 +287,9 @@ class TaskChangeNotificationController {
         if (listener instanceof Binder) {
             synchronized (mLocalTaskStackListeners) {
                 if (!mLocalTaskStackListeners.contains(listener)) {
+                    if (listener instanceof TaskStackListener) {
+                        ((TaskStackListener) listener).setIsLocal();
+                    }
                     mLocalTaskStackListeners.add(listener);
                 }
             }

services/core/java/com/android/server/wm/TaskSnapshotController.java

@@ -480,12 +480,17 @@ class TaskSnapshotController {
         }
         final HardwareBuffer buffer = screenshotBuffer == null ? null
                 : screenshotBuffer.getHardwareBuffer();
-        if (buffer == null || buffer.getWidth() <= 1 || buffer.getHeight() <= 1) {
+        if (isInvalidHardwareBuffer(buffer)) {
             return null;
         }
         return screenshotBuffer;
     }
 
+    static boolean isInvalidHardwareBuffer(HardwareBuffer buffer) {
+        return buffer == null || buffer.isClosed() // This must be checked before getting size.
+                || buffer.getWidth() <= 1 || buffer.getHeight() <= 1;
+    }
+
     @Nullable
     TaskSnapshot snapshotTask(Task task) {
         return snapshotTask(task, PixelFormat.UNKNOWN);

services/core/java/com/android/server/wm/TaskSnapshotPersister.java

@@ -407,6 +407,10 @@ class TaskSnapshotPersister {
         }
 
         boolean writeBuffer() {
+            if (TaskSnapshotController.isInvalidHardwareBuffer(mSnapshot.getHardwareBuffer())) {
+                Slog.e(TAG, "Invalid task snapshot hw buffer, taskId=" + mTaskId);
+                return false;
+            }
             final Bitmap bitmap = Bitmap.wrapHardwareBuffer(
                     mSnapshot.getHardwareBuffer(), mSnapshot.getColorSpace());
             if (bitmap == null) {

services/core/java/com/android/server/wm/WindowManagerService.java

@@ -9045,13 +9045,18 @@ public class WindowManagerService extends IWindowManager.Stub
         }
 
         TaskSnapshot taskSnapshot;
-        synchronized (mGlobalLock) {
-            Task task = mRoot.anyTaskForId(taskId, MATCH_ATTACHED_TASK_OR_RECENT_TASKS);
-            if (task == null) {
-                throw new IllegalArgumentException(
-                        "Failed to find matching task for taskId=" + taskId);
+        final long token = Binder.clearCallingIdentity();
+        try {
+            synchronized (mGlobalLock) {
+                Task task = mRoot.anyTaskForId(taskId, MATCH_ATTACHED_TASK_OR_RECENT_TASKS);
+                if (task == null) {
+                    throw new IllegalArgumentException(
+                            "Failed to find matching task for taskId=" + taskId);
+                }
+                taskSnapshot = mTaskSnapshotController.captureTaskSnapshot(task, false);
             }
-            taskSnapshot = mTaskSnapshotController.captureTaskSnapshot(task, false);
+        } finally {
+            Binder.restoreCallingIdentity(token);
         }
 
         if (taskSnapshot == null || taskSnapshot.getHardwareBuffer() == null) {

services/tests/wmtests/src/com/android/server/wm/TaskSnapshotPersisterLoaderTest.java

@@ -31,13 +31,13 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.when;
 
 import android.app.ActivityManager;
-import android.window.TaskSnapshot;
 import android.content.res.Configuration;
 import android.graphics.Rect;
 import android.os.SystemClock;
 import android.platform.test.annotations.Presubmit;
 import android.util.ArraySet;
 import android.view.Surface;
+import android.window.TaskSnapshot;
 
 import androidx.test.filters.MediumTest;
 
@@ -83,6 +83,12 @@ public class TaskSnapshotPersisterLoaderTest extends TaskSnapshotPersisterTestBa
         assertEquals(TEST_INSETS, snapshot.getContentInsets());
         assertNotNull(snapshot.getSnapshot());
         assertEquals(Configuration.ORIENTATION_PORTRAIT, snapshot.getOrientation());
+
+        snapshot.getHardwareBuffer().close();
+        mPersister.persistSnapshot(1, mTestUserId, snapshot);
+        mPersister.waitForQueueEmpty();
+        assertTrueForFiles(files, file -> !file.exists(),
+                " snapshot files must be removed by invalid buffer");
     }
 
     @Test

services/tests/wmtests/src/com/android/server/wm/TaskSnapshotPersisterTestBase.java

@@ -131,8 +131,7 @@ class TaskSnapshotPersisterTestBase extends WindowTestsBase {
     }
 
     TaskSnapshot createSnapshot() {
-        return new TaskSnapshotBuilder()
-                .build();
+        return new TaskSnapshotBuilder().setTopActivityComponent(getUniqueComponentName()).build();
     }
 
     protected static void assertTrueForFiles(File[] files, Predicate<File> predicate,

services/tests/wmtests/src/com/android/server/wm/WindowTestsBase.java

@@ -903,7 +903,7 @@ class WindowTestsBase extends SystemServiceTestsBase {
         doReturn(100).when(hardwareBuffer).getHeight();
     }
 
-    private static ComponentName getUniqueComponentName() {
+    static ComponentName getUniqueComponentName() {
         return ComponentName.createRelative(DEFAULT_COMPONENT_PACKAGE_NAME,
                 DEFAULT_COMPONENT_CLASS_NAME + sCurrentActivityId++);
     }