diff --git a/core/java/android/window/SurfaceSyncGroup.java b/core/java/android/window/SurfaceSyncGroup.java index 250652a536779..c0686fa4164d4 100644 --- a/core/java/android/window/SurfaceSyncGroup.java +++ b/core/java/android/window/SurfaceSyncGroup.java @@ -18,6 +18,7 @@ package android.window; import android.annotation.Nullable; import android.annotation.UiThread; +import android.os.Debug; import android.util.ArraySet; import android.util.Log; import android.util.Pair; @@ -141,7 +142,7 @@ public class SurfaceSyncGroup { }; if (DEBUG) { - Log.d(TAG, "setupSync"); + Log.d(TAG, "setupSync " + this + " " + Debug.getCallers(2)); } } @@ -313,10 +314,18 @@ public class SurfaceSyncGroup { // Additionally, the old parent will not get the final transaction object and // instead will send it to the new parent, ensuring that any other SurfaceSyncGroups // from the original parent are also combined with the new parent SurfaceSyncGroup. - if (mParentSyncGroup != null) { - Log.d(TAG, "Already part of sync group " + mParentSyncGroup + " " + this); + if (mParentSyncGroup != null && mParentSyncGroup != parentSyncGroup) { + if (DEBUG) { + Log.d(TAG, "Already part of sync group " + mParentSyncGroup + " " + this); + } parentSyncGroup.addToSync(mParentSyncGroup, true /* parentSyncGroupMerge */); } + + if (mParentSyncGroup == parentSyncGroup) { + if (DEBUG) { + Log.d(TAG, "Added to parent that was already the parent"); + } + } mParentSyncGroup = parentSyncGroup; final TransactionReadyCallback lastCallback = mTransactionReadyCallback; mTransactionReadyCallback = t -> { diff --git a/services/tests/wmtests/src/com/android/server/wm/SurfaceSyncGroupTest.java b/services/tests/wmtests/src/com/android/server/wm/SurfaceSyncGroupTest.java index 7d9f29c0a63db..266565946d93a 100644 --- a/services/tests/wmtests/src/com/android/server/wm/SurfaceSyncGroupTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/SurfaceSyncGroupTest.java @@ -336,6 +336,27 @@ public class SurfaceSyncGroupTest { verify(parentTransaction).merge(targetTransaction); } + @Test + public void testAddToSameParentNoCrash() { + final CountDownLatch finishedLatch = new CountDownLatch(1); + SurfaceSyncGroup syncGroup = new SurfaceSyncGroup(); + syncGroup.addSyncCompleteCallback(mExecutor, finishedLatch::countDown); + SyncTarget syncTarget = new SyncTarget(); + syncGroup.addToSync(syncTarget, false /* parentSyncGroupMerge */); + // Add the syncTarget to the same syncGroup and ensure it doesn't crash. + syncGroup.addToSync(syncTarget, false /* parentSyncGroupMerge */); + syncGroup.onTransactionReady(null); + + syncTarget.onBufferReady(); + + try { + finishedLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + assertEquals(0, finishedLatch.getCount()); + } + private static class SyncTarget extends SurfaceSyncGroup { void onBufferReady() { SurfaceControl.Transaction t = new StubTransaction();