From cdd84fe790b2b702ea9f0680d47f7f0176243175 Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Thu, 2 Feb 2023 07:01:09 +0000 Subject: [PATCH] Couple tweaks for surface control registry - Fix issue where invalid surface controls were being added to the registry but not removed - Update names for some places where SCs are created - Add sysprop to enable warning when SCs are built without a specified name - Add wm folks to test owners Bug: 266978825 Test: atest FrameworksCoreTests:android.view.SurfaceControlRegistryTests Change-Id: Ib6dbcd3e2d45329f7ea625a1eb57a9200c84bfdb Signed-off-by: Winson Chung --- core/java/android/view/SurfaceControl.java | 14 +++++++++++++- core/java/android/view/SurfaceControlViewHost.java | 3 +-- .../android/view/SurfaceControlRegistryTests.java | 12 ++++++++++++ .../java/com/android/server/wm/Transition.java | 2 ++ 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index ddaa71c6b1b59..c11f4975149d6 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -786,7 +786,11 @@ public final class SurfaceControl implements Parcelable { mReleaseStack = null; } setUnreleasedWarningCallSite(callsite); - addToRegistry(); + if (nativeObject != 0) { + // Only add valid surface controls to the registry. This is called at the end of this + // method since its information is dumped if the process threshold is reached. + addToRegistry(); + } } /** @@ -893,6 +897,10 @@ public final class SurfaceControl implements Parcelable { "Only buffer layers can set a valid buffer size."); } + if (mName == null) { + Log.w(TAG, "Missing name for SurfaceControl", new Throwable()); + } + if ((mFlags & FX_SURFACE_MASK) == FX_SURFACE_NORMAL) { setBLASTLayer(); } @@ -1254,6 +1262,9 @@ public final class SurfaceControl implements Parcelable { } /** + * Note: Most callers should use {@link SurfaceControl.Builder} or one of the other constructors + * to build an instance of a SurfaceControl. This constructor is mainly used for + * unparceling and passing into an AIDL call as an out parameter. * @hide */ public SurfaceControl() { @@ -2495,6 +2506,7 @@ public final class SurfaceControl implements Parcelable { public static SurfaceControl mirrorSurface(SurfaceControl mirrorOf) { long nativeObj = nativeMirrorSurface(mirrorOf.mNativeObject); SurfaceControl sc = new SurfaceControl(); + sc.mName = mirrorOf.mName + " (mirror)"; sc.assignNativeObject(nativeObj, "mirrorSurface"); return sc; } diff --git a/core/java/android/view/SurfaceControlViewHost.java b/core/java/android/view/SurfaceControlViewHost.java index d9872174f9bea..c8cf7d9a5194a 100644 --- a/core/java/android/view/SurfaceControlViewHost.java +++ b/core/java/android/view/SurfaceControlViewHost.java @@ -171,8 +171,7 @@ public class SurfaceControlViewHost { public SurfacePackage(@NonNull SurfacePackage other) { SurfaceControl otherSurfaceControl = other.mSurfaceControl; if (otherSurfaceControl != null && otherSurfaceControl.isValid()) { - mSurfaceControl = new SurfaceControl(); - mSurfaceControl.copyFrom(otherSurfaceControl, "SurfacePackage"); + mSurfaceControl = new SurfaceControl(otherSurfaceControl, "SurfacePackage"); } mAccessibilityEmbeddedConnection = other.mAccessibilityEmbeddedConnection; mInputToken = other.mInputToken; diff --git a/core/tests/coretests/src/android/view/SurfaceControlRegistryTests.java b/core/tests/coretests/src/android/view/SurfaceControlRegistryTests.java index d10ba7ccbac47..e117051ba9deb 100644 --- a/core/tests/coretests/src/android/view/SurfaceControlRegistryTests.java +++ b/core/tests/coretests/src/android/view/SurfaceControlRegistryTests.java @@ -103,6 +103,18 @@ public class SurfaceControlRegistryTests { assertEquals(hash0, SurfaceControlRegistry.getProcessInstance().hashCode()); } + @Test + public void testInvalidSurfaceControlNotAddedToRegistry() { + int hash0 = SurfaceControlRegistry.getProcessInstance().hashCode(); + // Verify no changes to the registry when dealing with invalid surface controls + SurfaceControl sc0 = new SurfaceControl(); + SurfaceControl sc1 = new SurfaceControl(sc0, "test"); + assertEquals(hash0, SurfaceControlRegistry.getProcessInstance().hashCode()); + sc0.release(); + sc1.release(); + assertEquals(hash0, SurfaceControlRegistry.getProcessInstance().hashCode()); + } + @Test public void testThresholds() { SurfaceControlRegistry registry = SurfaceControlRegistry.getProcessInstance(); diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index 0ce794fdb2ba9..72fd0c3c3cbe4 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -2234,8 +2234,10 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { while (leashReference.getParent() != ancestor) { leashReference = leashReference.getParent(); } + final SurfaceControl rootLeash = leashReference.makeAnimationLeash().setName( "Transition Root: " + leashReference.getName()).build(); + rootLeash.setUnreleasedWarningCallSite("Transition.calculateTransitionRoots"); startT.setLayer(rootLeash, leashReference.getLastLayer()); outInfo.addRootLeash(endDisplayId, rootLeash, ancestor.getBounds().left, ancestor.getBounds().top);