From 089c8193a31c9e28fcc4ed76ef3da7492e4b5dcc Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Fri, 17 May 2024 21:28:33 -0600 Subject: Make WindowContainer#mPendingTransaction optional This can reduce 70+ unused Transaction objects. So far the only used pending transaction is the instance in DisplayContent. The centralized transaction was to avoid applying unexpected order or incomplete surface operations. In general, it is not expected to care about the surface operations before a window container is attached. Because when a container is attached, the surface attributes will still be updated according to its current configuration. After this change, the mPendingTransaction of non-display container will only be used when creating display area of a new display. And then it will be null out immediately when attaching to display. Bug: 163976519 Bug: 132320879 Test: atest WindowContainerTests Change-Id: I7c2155bee55d260dbb72cf1f5f58c143f9dd03c6 --- services/core/java/com/android/server/wm/Task.java | 6 ++++++ .../java/com/android/server/wm/WindowContainer.java | 20 +++++++++++++------- .../com/android/server/wm/WindowContainerTests.java | 9 +++++---- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index 8bd7b5f78cf4..2c52eee00b99 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -1986,6 +1986,12 @@ class Task extends TaskFragment { final boolean wasInMultiWindowMode = inMultiWindowMode(); final boolean wasInPictureInPicture = inPinnedWindowingMode(); super.onConfigurationChanged(newParentConfig); + if (mDisplayContent == null) { + // This should be initializing from Task.Builder. The onConfigurationChanged will be + // called again when this task is attached to hierarchy. Early return here because the + // following operations are no-op for a non-attached task. + return; + } // Only need to update surface size here since the super method will handle updating // surface position. updateSurfaceSize(getSyncTransaction()); diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java index d70ca02cc23d..fadd77b3532b 100644 --- a/services/core/java/com/android/server/wm/WindowContainer.java +++ b/services/core/java/com/android/server/wm/WindowContainer.java @@ -201,8 +201,7 @@ class WindowContainer extends ConfigurationContainer< private int mLastLayer = 0; private SurfaceControl mLastRelativeToLayer = null; - // TODO(b/132320879): Remove this from WindowContainers except DisplayContent. - private final Transaction mPendingTransaction; + private Transaction mPendingTransaction; /** * Windows that clients are waiting to have drawn. @@ -357,7 +356,6 @@ class WindowContainer extends ConfigurationContainer< WindowContainer(WindowManagerService wms) { mWmService = wms; mTransitionController = mWmService.mAtmService.getTransitionController(); - mPendingTransaction = wms.mTransactionFactory.get(); mSyncTransaction = wms.mTransactionFactory.get(); mSurfaceAnimator = new SurfaceAnimator(this, this::onAnimationFinished, wms); mSurfaceFreezer = new SurfaceFreezer(this, wms); @@ -579,6 +577,10 @@ class WindowContainer extends ConfigurationContainer< @Override public void onConfigurationChanged(Configuration newParentConfig) { super.onConfigurationChanged(newParentConfig); + if (mParent == null) { + // Avoid unnecessary surface operation before attaching to a parent. + return; + } updateSurfacePositionNonOrganized(); scheduleAnimation(); if (mOverlayHost != null) { @@ -1073,8 +1075,9 @@ class WindowContainer extends ConfigurationContainer< } } mDisplayContent = dc; - if (dc != null && dc != this) { + if (dc != null && dc != this && mPendingTransaction != null) { dc.getPendingTransaction().merge(mPendingTransaction); + mPendingTransaction = null; } if (dc != this && mLocalInsetsSources != null) { mLocalInsetsSources.clear(); @@ -2923,14 +2926,17 @@ class WindowContainer extends ConfigurationContainer< @Override public Transaction getPendingTransaction() { - final DisplayContent displayContent = getDisplayContent(); - if (displayContent != null && displayContent != this) { - return displayContent.getPendingTransaction(); + final WindowContainer dc = mDisplayContent; + if (dc != null && dc.mPendingTransaction != null) { + return dc.mPendingTransaction; } // This WindowContainer has not attached to a display yet or this is a DisplayContent, so we // let the caller to save the surface operations within the local mPendingTransaction. // If this is not a DisplayContent, we will merge it to the pending transaction of its // display once it attaches to it. + if (mPendingTransaction == null) { + mPendingTransaction = mWmService.mTransactionFactory.get(); + } return mPendingTransaction; } diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java index 9f85acb98817..72e9eb394544 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java @@ -163,7 +163,7 @@ public class WindowContainerTests extends WindowTestsBase { @Test public void testAddChildSetsSurfacePosition() { reset(mTransaction); - try (MockSurfaceBuildingContainer top = new MockSurfaceBuildingContainer(mWm)) { + try (MockSurfaceBuildingContainer top = new MockSurfaceBuildingContainer(mDisplayContent)) { WindowContainer child = new WindowContainer(mWm); child.setBounds(1, 1, 10, 10); @@ -266,7 +266,7 @@ public class WindowContainerTests extends WindowTestsBase { @Test public void testRemoveImmediatelyClearsLastSurfacePosition() { reset(mTransaction); - try (MockSurfaceBuildingContainer top = new MockSurfaceBuildingContainer(mWm)) { + try (MockSurfaceBuildingContainer top = new MockSurfaceBuildingContainer(mDisplayContent)) { final WindowContainer child1 = new WindowContainer(mWm); child1.setBounds(1, 1, 10, 10); @@ -1814,8 +1814,9 @@ public class WindowContainerTests extends WindowTestsBase { implements AutoCloseable { private final SurfaceSession mSession = new SurfaceSession(); - MockSurfaceBuildingContainer(WindowManagerService wm) { - super(wm); + MockSurfaceBuildingContainer(DisplayContent dc) { + super(dc.mWmService); + onDisplayChanged(dc); } static class MockSurfaceBuilder extends SurfaceControl.Builder { -- cgit v1.2.3-59-g8ed1b