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

Commit 412f356e authored by Robert Carr's avatar Robert Carr
Browse files

More SurfaceAnimator lifetime fixes. 

1. The last fix to Dimmer was insufficient, the mDimLayer in the context
   of DimState() is not the mDimLayer that we return from DimAnimatable#getSurfaceControl.
   Here we null the correct layer.
2. This bug is hard to fix without repro so we add some isValid checks preventing further crashes
   at the cost of a few cycles and a code wart.

Test: Existing tests pass
Bug: 130315011
Bug: 130364451
Change-Id: I3d4d3e0e7110f943e5a2614e07a1f4ae85c2b3da
parent 8e3183e1

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

+7 −3
Original line number Diff line number Diff line
@@ -41,7 +41,7 @@ class Dimmer { 
    private static final int DEFAULT_DIM_ANIM_DURATION = 200;



    private class DimAnimatable implements SurfaceAnimator.Animatable {

        private final SurfaceControl mDimLayer;

        private SurfaceControl mDimLayer;



        private DimAnimatable(SurfaceControl dimLayer) {

            mDimLayer = dimLayer;
@@ -100,6 +100,11 @@ class Dimmer { 
            // See getSurfaceWidth() above for explanation.

            return mHost.getSurfaceHeight();

        }



        void removeSurface() {

            getPendingTransaction().remove(mDimLayer);

            mDimLayer = null;

        }

    }



    @VisibleForTesting
@@ -129,8 +134,7 @@ class Dimmer { 
            final DimAnimatable dimAnimatable = new DimAnimatable(dimLayer);

            mSurfaceAnimator = new SurfaceAnimator(dimAnimatable, () -> {

                if (!mDimming) {

                    dimAnimatable.getPendingTransaction().remove(mDimLayer);

                    mDimLayer = null;

                    dimAnimatable.removeSurface();

                }

            }, mHost.mWmService);

        }

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

+6 −2
Original line number Diff line number Diff line
@@ -286,9 +286,13 @@ class SurfaceAnimator { 
        final boolean destroy = mLeash != null && surface != null && parent != null;

        if (destroy) {

            if (DEBUG_ANIM) Slog.i(TAG, "Reparenting to original parent");

            // We shouldn't really need these isValid checks but we do

            // b/130364451

            if (surface.isValid() && parent.isValid()) {

                t.reparent(surface, parent);

                scheduleAnim = true;

            }

        }

        mService.mAnimationTransferMap.remove(mAnimation);

        if (mLeash != null && destroyLeash) {

            t.remove(mLeash);