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

Commit ff66c406 authored by Marzia Favaro's avatar Marzia Favaro
Browse files

Cancel dim animations directly from runner + verbose logs 

There have been unexplained NPE due to the access of released dim
surfaces from the animation. Cancel the animation directly from the
SurfaceAnimationRunner to reduc the likelihood of unsynchronized code
changing the state of the surface.

Track the most relevant lifecycle events to detect anomalies in surface
access in case this happens in the future. Currently this is not
debuggable when the issue is not easily reproducible, so we introduce
some verbose logging to be able to analyse the state of the dim at the
time of the crash

Test: DimmerTest
Bug: 377592988
Flag: EXEMPT logic is the same
Change-Id: I67ecd3b813015cd5bb5a247e5b7a20e8647f5e03
parent 0fb259da

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

+18 −4
Original line number Diff line number Diff line
@@ -17,8 +17,6 @@ 
package com.android.server.wm;



import static com.android.internal.protolog.WmProtoLogGroups.WM_DEBUG_DIMMER;

import static com.android.server.wm.WindowManagerDebugConfig.TAG_WITH_CLASS_NAME;

import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM;



import android.annotation.NonNull;

import android.annotation.Nullable;
@@ -44,7 +42,8 @@ class Dimmer { 
     */

    private final WindowContainer<?> mHost;



    private static final String TAG = TAG_WITH_CLASS_NAME ? "Dimmer" : TAG_WM;

    private static final String TAG = "WindowManagerDimmer";



    DimState mDimState;

    final DimmerAnimationHelper.AnimationAdapterFactory mAnimationAdapterFactory;
@@ -69,9 +68,10 @@ class Dimmer { 


        DimState() {

            mHostContainer = mHost;

            mAnimationHelper = new DimmerAnimationHelper(mAnimationAdapterFactory);

            mAnimationHelper = new DimmerAnimationHelper(mHost, mAnimationAdapterFactory);

            try {

                mDimSurface = makeDimLayer();

                EventLogTags.writeWmDimCreated(mHost.getName(), mDimSurface.getLayerId());

            } catch (Surface.OutOfResourcesException e) {

                Log.w(TAG, "OutOfResourcesException creating dim surface");

            }
@@ -102,6 +102,11 @@ class Dimmer { 
         * Prepare the dim for the exit animation

         */

        void exit(@NonNull SurfaceControl.Transaction t) {

            EventLogTags.writeWmDimExit(mDimState.mDimSurface.getLayerId(),

                    mDimState.mLastDimmingWindow != null

                            ? mDimState.mLastDimmingWindow.getName() : "-",

                    mDimState.mHostContainer.isVisible() ? 1 : 0,

                    mAnimateExit ? 0 : 1);

            if (!mAnimateExit) {

                remove(t);

            } else {
@@ -111,8 +116,10 @@ class Dimmer { 
        }



        void remove(@NonNull SurfaceControl.Transaction t) {

            EventLogTags.writeWmDimCancelAnim(mDimSurface.getLayerId(), "ready to remove");

            mAnimationHelper.stopCurrentAnimation(mDimSurface);

            if (mDimSurface.isValid()) {

                EventLogTags.writeWmDimRemoved(mDimSurface.getLayerId());

                t.remove(mDimSurface);

                ProtoLog.d(WM_DEBUG_DIMMER,

                        "Removing dim surface %s on transaction %s", this, t);
@@ -126,6 +133,13 @@ class Dimmer { 
            return "Dimmer#DimState with host=" + mHostContainer + ", surface=" + mDimSurface;

        }





        String reasonForRemoving() {

            return mLastDimmingWindow != null ? mLastDimmingWindow

                    + " is dimming but host " + mHostContainer + " is not visibleRequested"

                    : " no one is dimming";

        }



        /**

         * Set the parameters to prepare the dim to be relative parented to the dimming container

         */

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

+17 −16
Original line number Diff line number Diff line
@@ -76,10 +76,12 @@ public class DimmerAnimationHelper { 
            return mDimmingContainer != null && mDimmingContainer == other.mDimmingContainer;

        }



        void inheritPropertiesFromAnimation(@NonNull AnimationSpec anim) {

        void inheritPropertiesFromAnimation(@Nullable AnimationSpec anim) {

            if (anim != null) {

                mAlpha = anim.mCurrentAlpha;

                mBlurRadius = anim.mCurrentBlur;

            }

        }



        @Override

        public String toString() {
@@ -92,11 +94,13 @@ public class DimmerAnimationHelper { 
    private final Change mRequestedProperties = new Change();

    private AnimationSpec mAlphaAnimationSpec;



    private final SurfaceAnimationRunner mSurfaceAnimationRunner;

    private final AnimationAdapterFactory mAnimationAdapterFactory;

    private AnimationAdapter mLocalAnimationAdapter;



    DimmerAnimationHelper(AnimationAdapterFactory animationFactory) {

    DimmerAnimationHelper(WindowContainer<?> host, AnimationAdapterFactory animationFactory) {

        mAnimationAdapterFactory = animationFactory;

        mSurfaceAnimationRunner = host.mWmService.mSurfaceAnimationRunner;

    }



    void setExitParameters() {
@@ -160,6 +164,7 @@ public class DimmerAnimationHelper { 
        }



        if (!startProperties.hasSameVisualProperties(mRequestedProperties)) {

            EventLogTags.writeWmDimCancelAnim(dim.mDimSurface.getLayerId(), "new target values");

            stopCurrentAnimation(dim.mDimSurface);



            if (dim.mSkipAnimation
@@ -189,13 +194,15 @@ public class DimmerAnimationHelper { 
        ProtoLog.v(WM_DEBUG_DIMMER, "Starting animation on %s", dim);

        mAlphaAnimationSpec = getRequestedAnimationSpec(from, to);

        mLocalAnimationAdapter = mAnimationAdapterFactory.get(mAlphaAnimationSpec,

                dim.mHostContainer.mWmService.mSurfaceAnimationRunner);

                mSurfaceAnimationRunner);



        float targetAlpha = to.mAlpha;

        EventLogTags.writeWmDimAnimate(dim.mDimSurface.getLayerId(), targetAlpha, to.mBlurRadius);



        mLocalAnimationAdapter.startAnimation(dim.mDimSurface, t,

                ANIMATION_TYPE_DIMMER, /* finishCallback */ (type, animator) -> {

                    synchronized (dim.mHostContainer.mWmService.mGlobalLock) {

                        EventLogTags.writeWmDimFinishAnim(dim.mDimSurface.getLayerId());

                        SurfaceControl.Transaction finishTransaction =

                                dim.mHostContainer.getSyncTransaction();

                        setCurrentAlphaBlur(dim, finishTransaction);
@@ -208,19 +215,13 @@ public class DimmerAnimationHelper { 
                });

    }



    private boolean isAnimating() {

        return mAlphaAnimationSpec != null;

    }



    void stopCurrentAnimation(@NonNull SurfaceControl surface) {

        if (mLocalAnimationAdapter != null && isAnimating()) {

            // Save the current animation progress and cancel the animation

        // (If animating) save the current animation progress and cancel the animation

        mCurrentProperties.inheritPropertiesFromAnimation(mAlphaAnimationSpec);

            mLocalAnimationAdapter.onAnimationCancelled(surface);

        mSurfaceAnimationRunner.onAnimationCancelled(surface);

        mLocalAnimationAdapter = null;

        mAlphaAnimationSpec = null;

    }

    }



    @NonNull

    private static AnimationSpec getRequestedAnimationSpec(Change from, Change to) {

services/core/java/com/android/server/wm/EventLogTags.logtags

+13 −0
Original line number Diff line number Diff line
@@ -87,3 +87,16 @@ option java_package com.android.server.wm 


# Entering pip called

38000 wm_enter_pip (User|1|5),(Token|1|5),(Component Name|3),(is Auto Enter|3)



# Dim layer is created

38200 wm_dim_created (Host|3),(Surface|1)

# Dimmer is ready for removal

38201 wm_dim_exit (Surface|1),(dimmingWindow|3),(hostIsVisible|1),(removeImmediately|1)

# Dimmer is starting an animation

38202 wm_dim_animate (Surface|1, (toAlpha|5), (toBlur|5))

# Dimmer animation is cancelled

38203 wm_dim_cancel_anim (Surface|1),(reason|3)

# Dimmer animation is finished

38204 wm_dim_finish_anim (Surface|1)

# Dimmer removing surface

38205 wm_dim_removed (Surface|1)
 
 No newline at end of file