From 23c07addaf5a557b833589ad504ab7a79b173dab Mon Sep 17 00:00:00 2001 From: Evan Rosky Date: Mon, 11 Jan 2021 17:19:42 +0000 Subject: [PATCH] Create surfacecontrol before layout in relayoutWindow If there is no surfacecontrol, relayout is skipped. This is often fine because a later surfaceplacement pass usually happens; however, when combined with commitFinishDrawingLocked being out-of-order with layout during traversal, a race can happen where relayoutWindow creates the surface (after surface placement) and then commitFinishDrawing runs immediately after that causing a windowanimation to use non-laid-out surface positions. Bug: 174636007 Test: Use logging to verify that layout/surface-placement happen during relayoutWindow on a new window. Change-Id: Ic9d17e3d26de2ecd082e4fff642c1d456a7b071e --- .../server/wm/WindowManagerService.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 89d30408fe25c..07d04d7dcf0b6 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -2393,15 +2393,9 @@ public class WindowManagerService extends IWindowManager.Stub } } - // We may be deferring layout passes at the moment, but since the client is interested - // in the new out values right now we need to force a layout. - mWindowPlacerLocked.performSurfacePlacement(true /* force */); - + // Create surfaceControl before surface placement otherwise layout will be skipped + // (because WS.isGoneForLayout() is true when there is no surface. if (shouldRelayout) { - Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "relayoutWindow: viewVisibility_1"); - - result = win.relayoutVisibleWindow(result); - try { result = createSurfaceControl(outSurfaceControl, result, win, winAnimator); } catch (Exception e) { @@ -2413,6 +2407,17 @@ public class WindowManagerService extends IWindowManager.Stub Binder.restoreCallingIdentity(origId); return 0; } + } + + // We may be deferring layout passes at the moment, but since the client is interested + // in the new out values right now we need to force a layout. + mWindowPlacerLocked.performSurfacePlacement(true /* force */); + + if (shouldRelayout) { + Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "relayoutWindow: viewVisibility_1"); + + result = win.relayoutVisibleWindow(result); + if ((result & WindowManagerGlobal.RELAYOUT_RES_FIRST_TIME) != 0) { focusMayChange = true; }