From 84db17e81d9f1875b24cfdbbb435a0311bae751a Mon Sep 17 00:00:00 2001 From: Evan Rosky Date: Fri, 3 Feb 2023 00:44:25 -0800 Subject: [PATCH] Don't send transitions to Shell until finished booting While booting, the display isn't enabled yet so transition animations aren't visible. Additionally, there can be problems during this time because not all parts of the system are fully initialized yet causing animations to not run properly and potentially trigger ANRs. This CL "localizes" all transitions that start during the boot phase. This should be faster and prevents coupling WM and Shell processes while the system is already very busy. Bug: 267439165 Test: SystemAppCheck#system_app_crash Change-Id: If6625e57fcd2246835eaa5dc0155465631e8019c --- data/etc/services.core.protolog.json | 18 ++++++++++++ .../com/android/server/wm/Transition.java | 29 +++++++++++++++---- .../server/wm/TransitionController.java | 16 ++++++++++ .../server/wm/WindowManagerService.java | 7 +++++ .../server/wm/WindowOrganizerController.java | 1 + .../server/wm/SystemServicesTestRule.java | 1 + 6 files changed, 67 insertions(+), 5 deletions(-) diff --git a/data/etc/services.core.protolog.json b/data/etc/services.core.protolog.json index bccf2838fecc8..e9ed39671dbe4 100644 --- a/data/etc/services.core.protolog.json +++ b/data/etc/services.core.protolog.json @@ -61,6 +61,12 @@ "group": "WM_DEBUG_APP_TRANSITIONS", "at": "com\/android\/server\/wm\/WindowToken.java" }, + "-2088209279": { + "message": "Notified TransitionController that the display is ready.", + "level": "VERBOSE", + "group": "WM_DEBUG_WINDOW_TRANSITIONS", + "at": "com\/android\/server\/wm\/WindowManagerService.java" + }, "-2072089308": { "message": "Attempted to add window with token that is a sub-window: %s. Aborting.", "level": "WARN", @@ -2767,6 +2773,12 @@ "group": "WM_DEBUG_STATES", "at": "com\/android\/server\/wm\/TaskFragment.java" }, + "378890013": { + "message": "Apply and finish immediately because player is disabled for transition #%d .", + "level": "VERBOSE", + "group": "WM_DEBUG_WINDOW_TRANSITIONS", + "at": "com\/android\/server\/wm\/Transition.java" + }, "385237117": { "message": "moveFocusableActivityToTop: already on top and focused, activity=%s", "level": "DEBUG", @@ -3649,6 +3661,12 @@ "group": "WM_DEBUG_STATES", "at": "com\/android\/server\/wm\/ActivityRecord.java" }, + "1282992082": { + "message": "Disabling player for transition #%d because display isn't enabled yet", + "level": "VERBOSE", + "group": "WM_DEBUG_WINDOW_TRANSITIONS", + "at": "com\/android\/server\/wm\/TransitionController.java" + }, "1284122013": { "message": "TaskFragment appeared name=%s", "level": "VERBOSE", diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index d2f30ced46c8c..1a8671e91d0cc 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -216,6 +216,12 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { final TransitionController.Logger mLogger = new TransitionController.Logger(); + /** + * {@code false} if this transition runs purely in WMCore (meaning Shell is completely unaware + * of it). Currently, this happens before the display is ready since nothing can be seen yet. + */ + boolean mIsPlayerEnabled = true; + Transition(@TransitionType int type, @TransitionFlags int flags, TransitionController controller, BLASTSyncEngine syncEngine) { mType = type; @@ -766,7 +772,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { * be called directly; use {@link TransitionController#finishTransition} instead. */ void finishTransition() { - if (Trace.isTagEnabled(TRACE_TAG_WINDOW_MANAGER)) { + if (Trace.isTagEnabled(TRACE_TAG_WINDOW_MANAGER) && mIsPlayerEnabled) { Trace.asyncTraceEnd(TRACE_TAG_WINDOW_MANAGER, TRACE_NAME_PLAY_TRANSITION, System.identityHashCode(this)); } @@ -1096,7 +1102,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { controller.setupStartTransaction(transaction); } buildFinishTransaction(mFinishTransaction, info.getRootLeash()); - if (mController.getTransitionPlayer() != null) { + if (mController.getTransitionPlayer() != null && mIsPlayerEnabled) { mController.dispatchLegacyAppTransitionStarting(info, mStatusBarTransitionDelay); try { ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, @@ -1112,11 +1118,16 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { } catch (RemoteException e) { // If there's an exception when trying to send the mergedTransaction to the // client, we should finish and apply it here so the transactions aren't lost. - cleanUpOnFailure(); + postCleanupOnFailure(); } } else { - // No player registered, so just finish/apply immediately - cleanUpOnFailure(); + // No player registered or it's not enabled, so just finish/apply immediately + if (!mIsPlayerEnabled) { + mLogger.mSendTimeNs = SystemClock.uptimeNanos(); + ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Apply and finish immediately" + + " because player is disabled for transition #%d .", mSyncId); + } + postCleanupOnFailure(); } mController.mLoggerHandler.post(mLogger::logOnSend); mOverrideOptions = null; @@ -1127,6 +1138,14 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { info.releaseAnimSurfaces(); } + private void postCleanupOnFailure() { + mController.mAtm.mH.post(() -> { + synchronized (mController.mAtm.mGlobalLock) { + cleanUpOnFailure(); + } + }); + } + /** * If the remote failed for any reason, use this to do any appropriate clean-up. Do not call * this directly, it's designed to by called by {@link TransitionController} only. diff --git a/services/core/java/com/android/server/wm/TransitionController.java b/services/core/java/com/android/server/wm/TransitionController.java index 5e116baa3faec..a4475489e8e81 100644 --- a/services/core/java/com/android/server/wm/TransitionController.java +++ b/services/core/java/com/android/server/wm/TransitionController.java @@ -126,6 +126,13 @@ class TransitionController { final Handler mLoggerHandler = FgThread.getHandler(); + /** + * {@code true} While this waits for the display to become enabled (during boot). While waiting + * for the display, all core-initiated transitions will be "local". + * Note: This defaults to false so that it doesn't interfere with unit tests. + */ + boolean mIsWaitingForDisplayEnabled = false; + TransitionController(ActivityTaskManagerService atm, TaskSnapshotController taskSnapshotController, TransitionTracer transitionTracer) { @@ -460,6 +467,15 @@ class TransitionController { Transition requestStartTransition(@NonNull Transition transition, @Nullable Task startTask, @Nullable RemoteTransition remoteTransition, @Nullable TransitionRequestInfo.DisplayChange displayChange) { + if (mIsWaitingForDisplayEnabled) { + ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Disabling player for transition" + + " #%d because display isn't enabled yet", transition.getSyncId()); + transition.mIsPlayerEnabled = false; + transition.mLogger.mRequestTimeNs = SystemClock.uptimeNanos(); + mAtm.mH.post(() -> mAtm.mWindowOrganizerController.startTransition( + transition.getToken(), null)); + return transition; + } try { ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Requesting StartTransition: %s", transition); diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index b46a7203515ca..06a886924f043 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -310,6 +310,7 @@ import com.android.internal.policy.IKeyguardDismissCallback; import com.android.internal.policy.IKeyguardLockedStateListener; import com.android.internal.policy.IShortcutService; import com.android.internal.policy.KeyInterceptionInfo; +import com.android.internal.protolog.ProtoLogGroup; import com.android.internal.protolog.ProtoLogImpl; import com.android.internal.protolog.common.ProtoLog; import com.android.internal.util.DumpUtils; @@ -3776,6 +3777,12 @@ public class WindowManagerService extends IWindowManager.Stub // Make sure the last requested orientation has been applied. updateRotationUnchecked(false, false); + + synchronized (mGlobalLock) { + mAtmService.getTransitionController().mIsWaitingForDisplayEnabled = false; + ProtoLog.v(ProtoLogGroup.WM_DEBUG_WINDOW_TRANSITIONS, "Notified TransitionController " + + "that the display is ready."); + } } private boolean checkBootAnimationCompleteLocked() { diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java index abdc708ff7fca..497fc2f817d76 100644 --- a/services/core/java/com/android/server/wm/WindowOrganizerController.java +++ b/services/core/java/com/android/server/wm/WindowOrganizerController.java @@ -168,6 +168,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub void setWindowManager(WindowManagerService wms) { mTransitionController = new TransitionController(mService, wms.mTaskSnapshotController, wms.mTransitionTracer); + mTransitionController.mIsWaitingForDisplayEnabled = !wms.mDisplayEnabled; mTransitionController.registerLegacyListener(wms.mActivityManagerAppTransitionNotifier); } diff --git a/services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRule.java b/services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRule.java index 42bbd2d40f410..94193f4a2dd3b 100644 --- a/services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRule.java +++ b/services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRule.java @@ -352,6 +352,7 @@ public class SystemServicesTestRule implements TestRule { mAtmService.setWindowManager(mWmService); mWmService.mDisplayEnabled = true; mWmService.mDisplayReady = true; + mAtmService.getTransitionController().mIsWaitingForDisplayEnabled = false; // Set configuration for default display mWmService.getDefaultDisplayContentLocked().reconfigureDisplayLocked();