Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4130a68a authored by Jorim Jaggi's avatar Jorim Jaggi
Browse files

Fix SurfaceAnimator and SurfaceAnimationRunner tests 

Since we marked mAnimator.mInitialized to true in the tests,
WM executed things from another thread during tests leading to
concurrency bugs.

Instead, we stub out addAfterPrepareSurfacesRunnable to a consumer
which executes the runnable directly during tests, avoiding the
need to let WM process animation frames.

Also attempts to fix flakyness in SurfaceAnimationRunner

Test: go/wm-smoke
Test: SurfaceAnimatorTest
Test: SurfaceAnimationRunnerTest

Change-Id: Ic9522e1afef6ce62667aefca80e58d6fb1db3424
Fixes: 71650763
Fixes: 71602314
Bug: 71719744
parent 0d64cd33

services/core/java/com/android/server/wm/AppWindowThumbnail.java

+2 −1
Original line number Diff line number Diff line
@@ -49,7 +49,8 @@ class AppWindowThumbnail implements Animatable { 


    AppWindowThumbnail(Transaction t, AppWindowToken appToken, GraphicBuffer thumbnailHeader) {

        mAppToken = appToken;

        mSurfaceAnimator = new SurfaceAnimator(this, this::onAnimationFinished, appToken.mService);

        mSurfaceAnimator = new SurfaceAnimator(this, this::onAnimationFinished,

                appToken.mService.mAnimator::addAfterPrepareSurfacesRunnable, appToken.mService);

        mWidth = thumbnailHeader.getWidth();

        mHeight = thumbnailHeader.getHeight();

services/core/java/com/android/server/wm/SurfaceAnimator.java

+9 −4
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import android.view.SurfaceControl.Transaction; 
import com.android.internal.annotations.VisibleForTesting;



import java.io.PrintWriter;

import java.util.function.Consumer;



/**

 * A class that can run animations on objects that have a set of child surfaces. We do this by
@@ -55,16 +56,20 @@ class SurfaceAnimator { 
    /**

     * @param animatable The object to animate.

     * @param animationFinishedCallback Callback to invoke when an animation has finished running.

     * @param addAfterPrepareSurfaces Consumer that takes a runnable and executes it after preparing

     *                                surfaces in WM. Can be implemented differently during testing.

     */

    SurfaceAnimator(Animatable animatable, Runnable animationFinishedCallback,

            WindowManagerService service) {

            Consumer<Runnable> addAfterPrepareSurfaces, WindowManagerService service) {

        mAnimatable = animatable;

        mService = service;

        mAnimationFinishedCallback = animationFinishedCallback;

        mInnerAnimationFinishedCallback = getFinishedCallback(animationFinishedCallback);

        mInnerAnimationFinishedCallback = getFinishedCallback(animationFinishedCallback,

                addAfterPrepareSurfaces);

    }



    private OnAnimationFinishedCallback getFinishedCallback(Runnable animationFinishedCallback) {

    private OnAnimationFinishedCallback getFinishedCallback(Runnable animationFinishedCallback,

            Consumer<Runnable> addAfterPrepareSurfaces) {

        return anim -> {

            synchronized (mService.mWindowMap) {

                final SurfaceAnimator target = mService.mAnimationTransferMap.remove(anim);
@@ -80,7 +85,7 @@ class SurfaceAnimator { 
                // reparents the surface onto the leash is executed already. Otherwise this may be

                // executed first, leading to surface loss, as the reparent operations wouldn't

                // be in order.

                mService.mAnimator.addAfterPrepareSurfacesRunnable(() -> {

                addAfterPrepareSurfaces.accept(() -> {

                    if (anim != mAnimation) {

                        // Callback was from another animation - ignore.

                        return;

services/core/java/com/android/server/wm/WindowAnimator.java

+3 −5
Original line number Diff line number Diff line
@@ -47,7 +47,6 @@ public class WindowAnimator { 
    final WindowManagerService mService;

    final Context mContext;

    final WindowManagerPolicy mPolicy;

    private final WindowSurfacePlacer mWindowPlacerLocked;



    /** Is any window animating? */

    private boolean mAnimating;
@@ -74,7 +73,7 @@ public class WindowAnimator { 


    SparseArray<DisplayContentsAnimator> mDisplayContentsAnimators = new SparseArray<>(2);



    boolean mInitialized = false;

    private boolean mInitialized = false;



    // When set to true the animator will go over all windows after an animation frame is posted and

    // check if some got replaced and can be removed.
@@ -98,7 +97,6 @@ public class WindowAnimator { 
        mService = service;

        mContext = service.mContext;

        mPolicy = service.mPolicy;

        mWindowPlacerLocked = service.mWindowPlacerLocked;

        AnimationThread.getHandler().runWithScissors(

                () -> mChoreographer = Choreographer.getSfInstance(), 0 /* timeout */);
@@ -241,7 +239,7 @@ public class WindowAnimator { 
            }



            if (hasPendingLayoutChanges || doRequest) {

                mWindowPlacerLocked.requestTraversal();

                mService.mWindowPlacerLocked.requestTraversal();

            }



            final boolean rootAnimating = mService.mRoot.isSelfOrChildAnimating();
@@ -254,7 +252,7 @@ public class WindowAnimator { 
                Trace.asyncTraceBegin(Trace.TRACE_TAG_WINDOW_MANAGER, "animating", 0);

            }

            if (!rootAnimating && mLastRootAnimating) {

                mWindowPlacerLocked.requestTraversal();

                mService.mWindowPlacerLocked.requestTraversal();

                mService.mTaskSnapshotController.setPersisterPaused(false);

                Trace.asyncTraceEnd(Trace.TRACE_TAG_WINDOW_MANAGER, "animating", 0);

            }

services/core/java/com/android/server/wm/WindowContainer.java

+2 −1
Original line number Diff line number Diff line
@@ -102,7 +102,8 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer< 
    WindowContainer(WindowManagerService service) {

        mService = service;

        mPendingTransaction = service.mTransactionFactory.make();

        mSurfaceAnimator = new SurfaceAnimator(this, this::onAnimationFinished, service);

        mSurfaceAnimator = new SurfaceAnimator(this, this::onAnimationFinished,

                service.mAnimator::addAfterPrepareSurfacesRunnable, service);

    }



    @Override

services/core/java/com/android/server/wm/WindowManagerService.java

+4 −3
Original line number Diff line number Diff line
@@ -928,7 +928,6 @@ public class WindowManagerService extends IWindowManager.Stub 
            boolean haveInputMethods, boolean showBootMsgs, boolean onlyCore,

            WindowManagerPolicy policy) {

        installLock(this, INDEX_WINDOW);

        mRoot = new RootWindowContainer(this);

        mContext = context;

        mHaveInputMethods = haveInputMethods;

        mAllowBootMessages = showBootMsgs;
@@ -952,8 +951,11 @@ public class WindowManagerService extends IWindowManager.Stub 
        mDisplaySettings = new DisplaySettings();

        mDisplaySettings.readSettingsLocked();



        mWindowPlacerLocked = new WindowSurfacePlacer(this);

        mPolicy = policy;

        mAnimator = new WindowAnimator(this);

        mRoot = new RootWindowContainer(this);



        mWindowPlacerLocked = new WindowSurfacePlacer(this);

        mTaskSnapshotController = new TaskSnapshotController(this);



        mWindowTracing = WindowTracing.createDefaultAndStartLooper(context);
@@ -1051,7 +1053,6 @@ public class WindowManagerService extends IWindowManager.Stub 
                PowerManager.SCREEN_BRIGHT_WAKE_LOCK | PowerManager.ON_AFTER_RELEASE, TAG_WM);

        mHoldingScreenWakeLock.setReferenceCounted(false);



        mAnimator = new WindowAnimator(this);

        mSurfaceAnimationRunner = new SurfaceAnimationRunner();



        mAllowTheaterModeWakeFromLayout = context.getResources().getBoolean(