From c6a2b7e2995ff449a776a9899f6ff5a1b03773c0 Mon Sep 17 00:00:00 2001 From: Garfield Tan Date: Thu, 1 Sep 2022 11:45:08 -0700 Subject: [PATCH] Remove decoration layers from hierarchy on release Bug: 244586478 Bug: 243977698 Test: Decor layers aren't in the hierarchy after release. Test: Old task backgrounds don't remain. Change-Id: I28b31409069fd482bbf235ed39a16681e5b7c250 --- .../shell/windowdecor/WindowDecoration.java | 22 ++++++-- .../windowdecor/WindowDecorationTests.java | 51 ++++++++++++------- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/WindowDecoration.java b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/WindowDecoration.java index 2f41aa932d05a..3e3a864f48c73 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/WindowDecoration.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/WindowDecoration.java @@ -66,6 +66,7 @@ public abstract class WindowDecoration final DisplayController mDisplayController; final ShellTaskOrganizer mTaskOrganizer; final Supplier mSurfaceControlBuilderSupplier; + final Supplier mSurfaceControlTransactionSupplier; final Supplier mWindowContainerTransactionSupplier; final SurfaceControlViewHostFactory mSurfaceControlViewHostFactory; private final DisplayController.OnDisplaysChangedListener mOnDisplaysChangedListener = @@ -104,8 +105,8 @@ public abstract class WindowDecoration RunningTaskInfo taskInfo, SurfaceControl taskSurface) { this(context, displayController, taskOrganizer, taskInfo, taskSurface, - SurfaceControl.Builder::new, WindowContainerTransaction::new, - new SurfaceControlViewHostFactory() {}); + SurfaceControl.Builder::new, SurfaceControl.Transaction::new, + WindowContainerTransaction::new, new SurfaceControlViewHostFactory() {}); } WindowDecoration( @@ -115,6 +116,7 @@ public abstract class WindowDecoration RunningTaskInfo taskInfo, SurfaceControl taskSurface, Supplier surfaceControlBuilderSupplier, + Supplier surfaceControlTransactionSupplier, Supplier windowContainerTransactionSupplier, SurfaceControlViewHostFactory surfaceControlViewHostFactory) { mContext = context; @@ -123,6 +125,7 @@ public abstract class WindowDecoration mTaskInfo = taskInfo; mTaskSurface = taskSurface; mSurfaceControlBuilderSupplier = surfaceControlBuilderSupplier; + mSurfaceControlTransactionSupplier = surfaceControlTransactionSupplier; mWindowContainerTransactionSupplier = windowContainerTransactionSupplier; mSurfaceControlViewHostFactory = surfaceControlViewHostFactory; @@ -320,19 +323,28 @@ public abstract class WindowDecoration mCaptionWindowManager = null; + final SurfaceControl.Transaction t = mSurfaceControlTransactionSupplier.get(); + boolean released = false; if (mCaptionContainerSurface != null) { - mCaptionContainerSurface.release(); + t.remove(mCaptionContainerSurface); mCaptionContainerSurface = null; + released = true; } if (mDecorationContainerSurface != null) { - mDecorationContainerSurface.release(); + t.remove(mDecorationContainerSurface); mDecorationContainerSurface = null; + released = true; } if (mTaskBackgroundSurface != null) { - mTaskBackgroundSurface.release(); + t.remove(mTaskBackgroundSurface); mTaskBackgroundSurface = null; + released = true; + } + + if (released) { + t.apply(); } final WindowContainerTransaction wct = mWindowContainerTransactionSupplier.get(); diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/WindowDecorationTests.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/WindowDecorationTests.java index 1b74d7d0e9824..ab6ac949d4a35 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/WindowDecorationTests.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/WindowDecorationTests.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.any; import static org.mockito.Mockito.argThat; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.same; @@ -59,6 +60,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; import org.mockito.Mock; import java.util.ArrayList; @@ -96,6 +98,8 @@ public class WindowDecorationTests extends ShellTestCase { @Mock private WindowContainerTransaction mMockWindowContainerTransaction; + private final List mMockSurfaceControlTransactions = + new ArrayList<>(); private final List mMockSurfaceControlBuilders = new ArrayList<>(); private SurfaceControl.Transaction mMockSurfaceControlStartT; private SurfaceControl.Transaction mMockSurfaceControlFinishT; @@ -265,6 +269,9 @@ public class WindowDecorationTests extends ShellTestCase { createMockSurfaceControlBuilder(captionContainerSurface); mMockSurfaceControlBuilders.add(captionContainerSurfaceBuilder); + final SurfaceControl.Transaction t = mock(SurfaceControl.Transaction.class); + mMockSurfaceControlTransactions.add(t); + final ActivityManager.TaskDescription.Builder taskDescriptionBuilder = new ActivityManager.TaskDescription.Builder() .setBackgroundColor(Color.YELLOW); @@ -287,19 +294,19 @@ public class WindowDecorationTests extends ShellTestCase { windowDecor.relayout(taskInfo); verify(mMockSurfaceControlViewHost, never()).release(); - verify(decorContainerSurface, never()).release(); - verify(taskBackgroundSurface, never()).release(); - verify(captionContainerSurface, never()).release(); + verify(t, never()).apply(); verify(mMockWindowContainerTransaction, never()) .removeInsetsProvider(eq(taskInfo.token), any()); taskInfo.isVisible = false; windowDecor.relayout(taskInfo); - verify(mMockSurfaceControlViewHost).release(); - verify(decorContainerSurface).release(); - verify(taskBackgroundSurface).release(); - verify(captionContainerSurface).release(); + final InOrder releaseOrder = inOrder(t, mMockSurfaceControlViewHost); + releaseOrder.verify(mMockSurfaceControlViewHost).release(); + releaseOrder.verify(t).remove(captionContainerSurface); + releaseOrder.verify(t).remove(decorContainerSurface); + releaseOrder.verify(t).remove(taskBackgroundSurface); + releaseOrder.verify(t).apply(); verify(mMockWindowContainerTransaction).removeInsetsProvider(eq(taskInfo.token), any()); } @@ -351,21 +358,30 @@ public class WindowDecorationTests extends ShellTestCase { private TestWindowDecoration createWindowDecoration( ActivityManager.RunningTaskInfo taskInfo, SurfaceControl testSurface) { return new TestWindowDecoration(mContext, mMockDisplayController, mMockShellTaskOrganizer, - taskInfo, testSurface, new MockSurfaceControlBuilderSupplier(), + taskInfo, testSurface, + new MockObjectSupplier<>(mMockSurfaceControlBuilders, + () -> createMockSurfaceControlBuilder(mock(SurfaceControl.class))), + new MockObjectSupplier<>(mMockSurfaceControlTransactions, + () -> mock(SurfaceControl.Transaction.class)), () -> mMockWindowContainerTransaction, mMockSurfaceControlViewHostFactory); } - private class MockSurfaceControlBuilderSupplier implements Supplier { + private class MockObjectSupplier implements Supplier { + private final List mObjects; + private final Supplier mDefaultSupplier; private int mNumOfCalls = 0; + private MockObjectSupplier(List objects, Supplier defaultSupplier) { + mObjects = objects; + mDefaultSupplier = defaultSupplier; + } + @Override - public SurfaceControl.Builder get() { - final SurfaceControl.Builder builder = - mNumOfCalls < mMockSurfaceControlBuilders.size() - ? mMockSurfaceControlBuilders.get(mNumOfCalls) - : createMockSurfaceControlBuilder(mock(SurfaceControl.class)); + public T get() { + final T mock = mNumOfCalls < mObjects.size() + ? mObjects.get(mNumOfCalls) : mDefaultSupplier.get(); ++mNumOfCalls; - return builder; + return mock; } } @@ -383,11 +399,12 @@ public class WindowDecorationTests extends ShellTestCase { ShellTaskOrganizer taskOrganizer, ActivityManager.RunningTaskInfo taskInfo, SurfaceControl taskSurface, Supplier surfaceControlBuilderSupplier, + Supplier surfaceControlTransactionSupplier, Supplier windowContainerTransactionSupplier, SurfaceControlViewHostFactory surfaceControlViewHostFactory) { super(context, displayController, taskOrganizer, taskInfo, taskSurface, - surfaceControlBuilderSupplier, windowContainerTransactionSupplier, - surfaceControlViewHostFactory); + surfaceControlBuilderSupplier, surfaceControlTransactionSupplier, + windowContainerTransactionSupplier, surfaceControlViewHostFactory); } @Override