From ecae534c9dc7dc9dfa9231d8d010b60a1a464bb2 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Thu, 5 Jan 2023 01:16:09 +0800 Subject: [PATCH] Do not postpone draw transaction Otherwise if the operations are merged to mPostDrawTransaction when the buffer is requested to be synced, the transaction order may be unexpected. Then the layer may appear to be frozen and unable to handle subsequent operations, including removal. The activity window may have sync handler because it is transforming by SeamlessRotator. Reference changes: [1]: I00606c0ed250a10e1651c403665f39be52e9a89e [2]: Id89d039328bd9b091d4ed78a1616bc78d1bd83fa Since the reparenting for preserved surface is gone, it no longer needs a separated WSA#mPostDrawTransaction. The mPostDrawTransaction was used for [3]: Ie91da3c78eaf8fb52a21ae49ed190b42d92d0083 [4]: I14c2f7603345ad89f0af4db48150b05d8ada602a Bug: 263009910 Test: atest WindowStateTests#testApplyWithNextDraw Test: Disable auto rotation. Launch Youtube (disable PiP) and expand to landscape. Take screenshot and press edit (it should show in portrait). The screen is not hang when exiting the edit activity. Change-Id: I02eaa8a16153a12b4a605600e3c75beb38c6f5e0 --- .../com/android/server/wm/WindowState.java | 15 +--------- .../server/wm/WindowStateAnimator.java | 28 ++----------------- .../android/server/wm/WindowStateTests.java | 5 ++++ 3 files changed, 8 insertions(+), 40 deletions(-) diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 04699619bfc80..0e5b99dc54419 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -395,14 +395,6 @@ class WindowState extends WindowContainer implements WindowManagerP /** The last syncId associated with a BLAST prepareSync or 0 when no BLAST sync is active. */ int mPrepareSyncSeqId = 0; - /** - * {@code true} when the client was still drawing for sync when the sync-set was finished or - * cancelled. This can happen if the window goes away during a sync. In this situation we need - * to make sure to still apply the postDrawTransaction when it finishes to prevent the client - * from getting stuck in a bad state. - */ - boolean mClientWasDrawingForSync = false; - /** * Special mode that is intended only for the rounded corner overlay: during rotation * transition, we un-rotate the window token such that the window appears as it did before the @@ -6019,9 +6011,6 @@ class WindowState extends WindowContainer implements WindowManagerP @Override void finishSync(Transaction outMergedTransaction, boolean cancel) { - if (mSyncState == SYNC_STATE_WAITING_FOR_DRAW && mRedrawForSyncReported) { - mClientWasDrawingForSync = true; - } mPrepareSyncSeqId = 0; if (cancel) { // This is leaving sync so any buffers left in the sync have a chance of @@ -6089,9 +6078,7 @@ class WindowState extends WindowContainer implements WindowManagerP layoutNeeded = onSyncFinishedDrawing(); } - layoutNeeded |= - mWinAnimator.finishDrawingLocked(postDrawTransaction, mClientWasDrawingForSync); - mClientWasDrawingForSync = false; + layoutNeeded |= mWinAnimator.finishDrawingLocked(postDrawTransaction); // We always want to force a traversal after a finish draw for blast sync. return !skipLayout && (hasSyncHandlers || layoutNeeded); } diff --git a/services/core/java/com/android/server/wm/WindowStateAnimator.java b/services/core/java/com/android/server/wm/WindowStateAnimator.java index a0ba8fda906f8..f3642480da58a 100644 --- a/services/core/java/com/android/server/wm/WindowStateAnimator.java +++ b/services/core/java/com/android/server/wm/WindowStateAnimator.java @@ -151,17 +151,6 @@ class WindowStateAnimator { int mAttrType; - /** - * Handles surface changes synchronized to after the client has drawn the surface. This - * transaction is currently used to reparent the old surface children to the new surface once - * the client has completed drawing to the new surface. - * This transaction is also used to merge transactions parceled in by the client. The client - * uses the transaction to update the relative z of its children from the old parent surface - * to the new parent surface once window manager reparents its children. - */ - private final SurfaceControl.Transaction mPostDrawTransaction = - new SurfaceControl.Transaction(); - WindowStateAnimator(final WindowState win) { final WindowManagerService service = win.mWmService; @@ -217,8 +206,7 @@ class WindowStateAnimator { } } - boolean finishDrawingLocked(SurfaceControl.Transaction postDrawTransaction, - boolean forceApplyNow) { + boolean finishDrawingLocked(SurfaceControl.Transaction postDrawTransaction) { final boolean startingWindow = mWin.mAttrs.type == WindowManager.LayoutParams.TYPE_APPLICATION_STARTING; if (startingWindow) { @@ -240,14 +228,7 @@ class WindowStateAnimator { } if (postDrawTransaction != null) { - // If there is no surface, the last draw was for the previous surface. We don't want to - // wait until the new surface is shown and instead just apply the transaction right - // away. - if (mLastHidden && mDrawState != NO_SURFACE && !forceApplyNow) { - mPostDrawTransaction.merge(postDrawTransaction); - } else { - mWin.getSyncTransaction().merge(postDrawTransaction); - } + mWin.getSyncTransaction().merge(postDrawTransaction); layoutNeeded = true; } @@ -547,7 +528,6 @@ class WindowStateAnimator { if (!shown) return false; - t.merge(mPostDrawTransaction); return true; } @@ -714,10 +694,6 @@ class WindowStateAnimator { } void destroySurface(SurfaceControl.Transaction t) { - // Since the SurfaceControl is getting torn down, it's safe to just clean up any - // pending transactions that were in mPostDrawTransaction, as well. - t.merge(mPostDrawTransaction); - try { if (mSurfaceController != null) { mSurfaceController.destroy(t); diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java index 0568f2acf366e..d83bfc2b89e50 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java @@ -544,7 +544,12 @@ public class WindowStateTests extends WindowTestsBase { win.applyWithNextDraw(t -> handledT[0] = t); assertTrue(win.useBLASTSync()); final SurfaceControl.Transaction drawT = new StubTransaction(); + final SurfaceControl.Transaction currT = win.getSyncTransaction(); + clearInvocations(currT); + win.mWinAnimator.mLastHidden = true; assertTrue(win.finishDrawing(drawT, Integer.MAX_VALUE)); + // The draw transaction should be merged to current transaction even if the state is hidden. + verify(currT).merge(eq(drawT)); assertEquals(drawT, handledT[0]); assertFalse(win.useBLASTSync());