From 355befde244080023631eba3f553471f05de6fd4 Mon Sep 17 00:00:00 2001 From: chaviw Date: Tue, 23 Jun 2020 08:11:46 -0700 Subject: [PATCH] Call finishDrawing and notify when Window is removed or hidden There are cases where a sync transaction is called on a window that's going to be removed or hidden. In that case, it will never get a finishDrawing and possibly not get a prepareSurfaces call. Since we're not really waiting for anything, we can call finishDrawing and immediately call notifyBlastSyncTransaction. This change also moves the blast sync timeout removal into notifyBlastSyncTransaction in case there's another issue where finishDrawing is called but not notifyBlastSyncTransaction. The reason for this is the device can at least go back to a normal state instead of permanently staying stuck. Fixes: 159507259 Test: WindowOrganizerTests Change-Id: If483e3769a98cc100c86893f2132aced5f064fe6 --- .../android/server/wm/WindowManagerService.java | 4 ++-- .../server/wm/WindowOrganizerController.java | 3 +++ .../java/com/android/server/wm/WindowState.java | 15 ++++++++++----- .../android/server/wm/WindowOrganizerTests.java | 17 ++++++----------- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 27864393d23e5..0539245683654 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -5154,8 +5154,8 @@ public class WindowManagerService extends IWindowManager.Stub } case WINDOW_STATE_BLAST_SYNC_TIMEOUT: { synchronized (mGlobalLock) { - final WindowState ws = (WindowState) msg.obj; - ws.finishDrawing(null); + final WindowState ws = (WindowState) msg.obj; + ws.immediatelyNotifyBlastSync(); } break; } diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java index fbc5afadac6ba..46e1bf04bdbe7 100644 --- a/services/core/java/com/android/server/wm/WindowOrganizerController.java +++ b/services/core/java/com/android/server/wm/WindowOrganizerController.java @@ -428,6 +428,9 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub try { callback.onTransactionReady(mSyncId, mergedTransaction); } catch (RemoteException e) { + // If there's an exception when trying to send the mergedTransaction to the client, we + // should immediately apply it here so the transactions aren't lost. + mergedTransaction.apply(); } mTransactionCallbacksByPendingSyncId.remove(mSyncId); diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 26a1fea1732b3..49ef4e46bfeb5 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -2192,7 +2192,7 @@ class WindowState extends WindowContainer implements WindowManagerP void removeIfPossible() { super.removeIfPossible(); removeIfPossible(false /*keepVisibleDeadWindow*/); - finishDrawing(null); + immediatelyNotifyBlastSync(); } private void removeIfPossible(boolean keepVisibleDeadWindow) { @@ -5806,7 +5806,7 @@ class WindowState extends WindowContainer implements WindowManagerP // client will not render when visibility is GONE. Therefore, call finishDrawing here to // prevent system server from blocking on a window that will not draw. if (viewVisibility == View.GONE && mUsingBLASTSyncTransaction) { - finishDrawing(null); + immediatelyNotifyBlastSync(); } } @@ -5844,7 +5844,6 @@ class WindowState extends WindowContainer implements WindowManagerP return mWinAnimator.finishDrawingLocked(postDrawTransaction); } - mWmService.mH.removeMessages(WINDOW_STATE_BLAST_SYNC_TIMEOUT, this); if (postDrawTransaction != null) { mBLASTSyncTransaction.merge(postDrawTransaction); } @@ -5853,8 +5852,9 @@ class WindowState extends WindowContainer implements WindowManagerP return mWinAnimator.finishDrawingLocked(null); } - @VisibleForTesting - void notifyBlastSyncTransaction() { + private void notifyBlastSyncTransaction() { + mWmService.mH.removeMessages(WINDOW_STATE_BLAST_SYNC_TIMEOUT, this); + if (!mNotifyBlastOnSurfacePlacement || mWaitingListener == null) { mNotifyBlastOnSurfacePlacement = false; return; @@ -5877,6 +5877,11 @@ class WindowState extends WindowContainer implements WindowManagerP mNotifyBlastOnSurfacePlacement = false; } + void immediatelyNotifyBlastSync() { + finishDrawing(null); + notifyBlastSyncTransaction(); + } + private boolean requestResizeForBlastSync() { return useBLASTSync() && !mResizeForBlastSyncReported; } diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowOrganizerTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowOrganizerTests.java index 7ce0c1edfb4c4..341e20946c300 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowOrganizerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowOrganizerTests.java @@ -729,7 +729,7 @@ public class WindowOrganizerTests extends WindowTestsBase { // We should be rejected from the second sync since we are already // in one. assertEquals(false, bse.addToSyncSet(id2, task)); - finishAndNotifyDrawing(w); + w.immediatelyNotifyBlastSync(); assertEquals(true, bse.addToSyncSet(id2, task)); bse.setReady(id2); } @@ -753,7 +753,7 @@ public class WindowOrganizerTests extends WindowTestsBase { // Since we have a window we have to wait for it to draw to finish sync. verify(transactionListener, never()) .onTransactionReady(anyInt(), any()); - finishAndNotifyDrawing(w); + w.immediatelyNotifyBlastSync(); verify(transactionListener) .onTransactionReady(anyInt(), any()); } @@ -821,14 +821,14 @@ public class WindowOrganizerTests extends WindowTestsBase { int id = bse.startSyncSet(transactionListener); assertEquals(true, bse.addToSyncSet(id, task)); bse.setReady(id); - finishAndNotifyDrawing(w); + w.immediatelyNotifyBlastSync(); // Since we have a child window we still shouldn't be done. verify(transactionListener, never()) .onTransactionReady(anyInt(), any()); reset(transactionListener); - finishAndNotifyDrawing(child); + child.immediatelyNotifyBlastSync(); // Ah finally! Done verify(transactionListener) .onTransactionReady(anyInt(), any()); @@ -1002,20 +1002,15 @@ public class WindowOrganizerTests extends WindowTestsBase { verify(mockCallback, never()).onTransactionReady(anyInt(), any()); assertTrue(w1.useBLASTSync()); assertTrue(w2.useBLASTSync()); - finishAndNotifyDrawing(w1); + w1.immediatelyNotifyBlastSync(); // Even though one Window finished drawing, both windows should still be using blast sync assertTrue(w1.useBLASTSync()); assertTrue(w2.useBLASTSync()); - finishAndNotifyDrawing(w2); + w2.immediatelyNotifyBlastSync(); verify(mockCallback).onTransactionReady(anyInt(), any()); assertFalse(w1.useBLASTSync()); assertFalse(w2.useBLASTSync()); } - - private void finishAndNotifyDrawing(WindowState ws) { - ws.finishDrawing(null); - ws.notifyBlastSyncTransaction(); - } }