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

Commit ca6ad70c authored by Liana Kazanova (xWF)'s avatar Liana Kazanova (xWF) Committed by Ming-Shin Lu
Browse files

Reapply "Fix perf regression VisualStabilityCoordinator" 

This reverts commit e639f542.

Previously revering CL[1] to fix CL[2] made
simpleSmartReplyWithLifetimeExtension_matchesScreenshot as presubmit
because the CL[1] breaks the notification post by visibility statbility
coordiator received mLockscreenInGoneTransition during unlocking to
pending the notification pipeline but never reset it.

The issue only reproducible when Scene container & keyguard WM refactor
flags (flexi) enabled, and the root cause is in SceneContainerStartable
didn't invoke statusBarKeyguardViewManager.hide() ->
setKeyguardFadingAway(true) when the keyguard transiting
to gone in the first place, caused hide() end up being consumed after
the keyguard visiblity changed & makes setKeyguardFadingAway(true)
invoked after keyguard unlocked without reset it.

Moving forward, for flexi will no longer rely on KeyguardStateController callback, as a result, we use SceneContainer#isEnabled as a check to use KeyguardStateController legacy callback for non-flexi to fix this perf regression as a short-term and keep monitoring keyguard gone trancition by alwaysCollectFlow for flexi enabled (and file b/381779933 for tracking potential perf regression for flexi)

With that, we can safely repplay the previous reverts CL back.

[1]: I14eadece21d48442581d75a7edbb2b383832003d
[2]: I71bb0969113dd12ec5ea0f04c79e016b672795bd

Flag: com.android.systemui.check_lockscreen_gone_transition
Bug: 375302455
Bug: 373203637
Bug: 373169481
Test: atest SystemUIMicrobenchmark
Test: atest VisualStabilityCoordinatorTest
Test: atest NotificationRemoteInput
      (with enabling Scene container & keyguard WM refactor flags)
Change-Id: Ibbcf21845cb0a87e91117cecc25bc87fa785607c
parent 5e9b7e14

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinatorTest.java

+25 −0
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import static org.junit.Assert.assertTrue; 
import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;
@@ -45,6 +46,8 @@ import com.android.systemui.SysuiTestCase; 
import com.android.systemui.communal.shared.model.CommunalScenes;

import com.android.systemui.dump.DumpManager;

import com.android.systemui.flags.BrokenWithSceneContainer;

import com.android.systemui.flags.DisableSceneContainer;

import com.android.systemui.flags.EnableSceneContainer;

import com.android.systemui.keyguard.WakefulnessLifecycle;

import com.android.systemui.keyguard.shared.model.KeyguardState;

import com.android.systemui.keyguard.shared.model.TransitionState;
@@ -68,6 +71,7 @@ import com.android.systemui.statusbar.notification.collection.listbuilder.plugga 
import com.android.systemui.statusbar.notification.collection.provider.VisualStabilityProvider;

import com.android.systemui.statusbar.notification.domain.interactor.SeenNotificationsInteractor;

import com.android.systemui.statusbar.notification.headsup.HeadsUpManager;

import com.android.systemui.statusbar.policy.KeyguardStateController;

import com.android.systemui.util.concurrency.FakeExecutor;

import com.android.systemui.util.kotlin.JavaAdapter;

import com.android.systemui.util.time.FakeSystemClock;
@@ -110,6 +114,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 
    @Mock private VisibilityLocationProvider mVisibilityLocationProvider;

    @Mock private VisualStabilityProvider mVisualStabilityProvider;

    @Mock private VisualStabilityCoordinatorLogger mLogger;

    @Mock private KeyguardStateController mKeyguardStateController;



    @Captor private ArgumentCaptor<WakefulnessLifecycle.Observer> mWakefulnessObserverCaptor;

    @Captor private ArgumentCaptor<StatusBarStateController.StateListener> mSBStateListenerCaptor;
@@ -155,6 +160,7 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 
                mKosmos.getCommunalSceneInteractor(),

                mKosmos.getShadeInteractor(),

                mKosmos.getKeyguardTransitionInteractor(),

                mKeyguardStateController,

                mLogger);

        mCoordinator.attach(mNotifPipeline);

        mTestScope.getTestScheduler().runCurrent();
@@ -539,6 +545,25 @@ public class VisualStabilityCoordinatorTest extends SysuiTestCase { 


    @Test

    @EnableFlags(Flags.FLAG_CHECK_LOCKSCREEN_GONE_TRANSITION)

    @DisableSceneContainer

    public void testNotLockscreenInGoneTransitionLegacy_invalidationCalled() {

        // GIVEN visual stability is being maintained b/c animation is playing

        doReturn(true).when(mKeyguardStateController).isKeyguardFadingAway();

        mCoordinator.mKeyguardFadeAwayAnimationCallback.onKeyguardFadingAwayChanged();



        assertFalse(mNotifStabilityManager.isPipelineRunAllowed());



        // WHEN the animation has stopped playing

        doReturn(false).when(mKeyguardStateController).isKeyguardFadingAway();

        mCoordinator.mKeyguardFadeAwayAnimationCallback.onKeyguardFadingAwayChanged();



        // invalidate is called, b/c we were previously suppressing the pipeline from running

        verifyStabilityManagerWasInvalidated(times(1));

    }



    @Test

    @EnableFlags(Flags.FLAG_CHECK_LOCKSCREEN_GONE_TRANSITION)

    @EnableSceneContainer

    @BrokenWithSceneContainer(bugId = 377868472) // mReorderingAllowed is broken with SceneContainer

    public void testNotLockscreenInGoneTransition_invalidationCalled() {

        // GIVEN visual stability is being maintained b/c animation is playing

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/VisualStabilityCoordinator.java

+21 −5
Original line number Diff line number Diff line
@@ -46,6 +46,7 @@ import com.android.systemui.statusbar.notification.collection.provider.VisualSta 
import com.android.systemui.statusbar.notification.domain.interactor.SeenNotificationsInteractor;

import com.android.systemui.statusbar.notification.shared.NotificationMinimalism;

import com.android.systemui.statusbar.notification.headsup.HeadsUpManager;

import com.android.systemui.statusbar.policy.KeyguardStateController;

import com.android.systemui.util.concurrency.DelayableExecutor;

import com.android.systemui.util.kotlin.BooleanFlowOperators;

import com.android.systemui.util.kotlin.JavaAdapter;
@@ -78,6 +79,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable { 
    private final CommunalSceneInteractor mCommunalSceneInteractor;

    private final ShadeInteractor mShadeInteractor;

    private final KeyguardTransitionInteractor mKeyguardTransitionInteractor;

    private final KeyguardStateController mKeyguardStateController;

    private final VisualStabilityCoordinatorLogger mLogger;



    private boolean mSleepy = true;
@@ -120,6 +122,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable { 
            CommunalSceneInteractor communalSceneInteractor,

            ShadeInteractor shadeInteractor,

            KeyguardTransitionInteractor keyguardTransitionInteractor,

            KeyguardStateController keyguardStateController,

            VisualStabilityCoordinatorLogger logger) {

        mHeadsUpManager = headsUpManager;

        mShadeAnimationInteractor = shadeAnimationInteractor;
@@ -133,6 +136,7 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable { 
        mCommunalSceneInteractor = communalSceneInteractor;

        mShadeInteractor = shadeInteractor;

        mKeyguardTransitionInteractor = keyguardTransitionInteractor;

        mKeyguardStateController = keyguardStateController;

        mLogger = logger;



        dumpManager.registerDumpable(this);
@@ -162,17 +166,29 @@ public class VisualStabilityCoordinator implements Coordinator, Dumpable { 
                            KeyguardState.LOCKSCREEN),

                    this::onLockscreenKeyguardStateTransitionValueChanged);

        }



        if (Flags.checkLockscreenGoneTransition()) {

            if (SceneContainerFlag.isEnabled()) {

                mJavaAdapter.alwaysCollectFlow(mKeyguardTransitionInteractor.isInTransition(

                            Edge.create(KeyguardState.LOCKSCREEN, Scenes.Gone),

                            Edge.create(KeyguardState.LOCKSCREEN, KeyguardState.GONE)),

                                Edge.create(KeyguardState.LOCKSCREEN, Scenes.Gone), null),

                        this::onLockscreenInGoneTransitionChanged);

            } else {

                mKeyguardStateController.addCallback(mKeyguardFadeAwayAnimationCallback);

            }

        }





        pipeline.setVisualStabilityManager(mNotifStabilityManager);

    }



    final KeyguardStateController.Callback mKeyguardFadeAwayAnimationCallback =

            new KeyguardStateController.Callback() {

                @Override

                public void onKeyguardFadingAwayChanged() {

                    onLockscreenInGoneTransitionChanged(

                            mKeyguardStateController.isKeyguardFadingAway());

                }

            };



    // TODO(b/203826051): Ensure stability manager can allow reordering off-screen

    //  HUNs to the top of the shade

    private final NotifStabilityManager mNotifStabilityManager =