From c003cb87851950d8aa5934e56fb53aa120390bbf Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Tue, 24 May 2022 18:12:26 +0000 Subject: [PATCH] Revert suppression of recalculation to fix jump after aod->ls animation The change I6575f733b17d6c8905150c0a7854d752dd0a5cfe suppressed recalculation of the number of notifications whenever dozeAmount != 0. However, this meant that from the time the device was locked to the time it finished the screen on animation, the number of notifications that fit on the device was never updated. In cases where notifications changed while on aod, this meant the number could be completely wrong. But even without that, this meant that when locking the device and then unlocking, the count would be based on the height of the notifications given their non-keyguard expansion state (i.e. first notification expanded) which meant that we almost always showed too few notifications during the aod->ls transition. I am unable to reproduce b/229882633 (which that CL was fixing) until I also revert I139d25e2bb57257a18f18f6288f3e18b7a1b72d1 which landed a week later. Bug: 229882633 Fixes: 232079794 Test: atest NotificationPanelViewControllerTest Test: numerous failed attempts to reproduce b/229882633#comment2 Test: unlock; screen off (to aod); screen on (to ls); observe no jumps Test: post notify notifications with "no text" ; slow animations; lock; aod -> ls; post notifications via shell during animation; observe no jumps during or at end of animation Change-Id: Ib3a3c6377a71cc4c1a1b06eb652237a5493a56d0 --- .../stack/NotificationStackSizeCalculator.kt | 4 ++- .../NotificationPanelViewController.java | 26 +++++++++++++++++-- .../NotificationPanelViewControllerTest.java | 11 -------- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt index d05c3385e982e..6287857e7be97 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt @@ -37,6 +37,7 @@ import kotlin.properties.Delegates.notNull private const val TAG = "NotifStackSizeCalc" private val DEBUG = Compile.IS_DEBUG && Log.isLoggable(TAG, Log.DEBUG) +private val SPEW = Compile.IS_DEBUG && Log.isLoggable(TAG, Log.VERBOSE) /** Calculates number of notifications to display and the height of the notification stack. */ @SysUISingleton @@ -87,9 +88,10 @@ constructor( // Could be < 0 if the space available is less than the shelf size. Returns 0 in this case. maxNotifications = max(0, maxNotifications) log { + val sequence = if (SPEW) " stackHeightSequence=${stackHeightSequence.toList()}" else "" "computeMaxKeyguardNotifications(" + "availableSpace=$totalAvailableSpace" + - " shelfHeight=$shelfIntrinsicHeight) -> $maxNotifications" + " shelfHeight=$shelfIntrinsicHeight) -> $maxNotifications$sequence" } return maxNotifications } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java index a9725db32ee9d..9afdfd6511307 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java @@ -192,6 +192,7 @@ import com.android.systemui.statusbar.policy.KeyguardUserSwitcherView; import com.android.systemui.statusbar.policy.OnHeadsUpChangedListener; import com.android.systemui.statusbar.window.StatusBarWindowStateController; import com.android.systemui.unfold.SysUIUnfoldComponent; +import com.android.systemui.util.Compile; import com.android.systemui.util.LargeScreenUtils; import com.android.systemui.util.ListenerSet; import com.android.systemui.util.Utils; @@ -216,7 +217,8 @@ import javax.inject.Provider; @CentralSurfacesComponent.CentralSurfacesScope public class NotificationPanelViewController extends PanelViewController { - private static final boolean DEBUG_LOGCAT = Log.isLoggable(TAG, Log.DEBUG); + private static final boolean DEBUG_LOGCAT = Compile.IS_DEBUG && Log.isLoggable(TAG, Log.DEBUG); + private static final boolean SPEW_LOGCAT = Compile.IS_DEBUG && Log.isLoggable(TAG, Log.VERBOSE); private static final boolean DEBUG_DRAWABLE = false; /** @@ -1294,6 +1296,8 @@ public class NotificationPanelViewController extends PanelViewController { private void updateMaxDisplayedNotifications(boolean recompute) { if (recompute) { mMaxAllowedKeyguardNotifications = Math.max(computeMaxKeyguardNotifications(), 1); + } else { + if (SPEW_LOGCAT) Log.d(TAG, "Skipping computeMaxKeyguardNotifications() by request"); } if (mKeyguardShowing && !mKeyguardBypassController.getBypassEnabled()) { @@ -1546,6 +1550,19 @@ public class NotificationPanelViewController extends PanelViewController { mNotificationStackScrollLayoutController.getHeight() - staticTopPadding - bottomPadding; + + if (SPEW_LOGCAT) { + Log.d(TAG, "getSpaceForLockscreenNotifications()" + + " availableSpace=" + availableSpace + + " NSSL.height=" + mNotificationStackScrollLayoutController.getHeight() + + " NSSL.top=" + mNotificationStackScrollLayoutController.getTop() + + " staticTopPadding=" + staticTopPadding + + " bottomPadding=" + bottomPadding + + " lockIconPadding=" + lockIconPadding + + " mIndicationBottomPadding=" + mIndicationBottomPadding + + " mAmbientIndicationBottomPadding=" + mAmbientIndicationBottomPadding + ); + } return availableSpace; } @@ -1554,7 +1571,12 @@ public class NotificationPanelViewController extends PanelViewController { */ @VisibleForTesting int computeMaxKeyguardNotifications() { - if (mAmbientState.getFractionToShade() > 0 || mAmbientState.getDozeAmount() > 0) { + if (mAmbientState.getFractionToShade() > 0) { + if (SPEW_LOGCAT) { + Log.v(TAG, "Internally skipping computeMaxKeyguardNotifications()" + + " fractionToShade=" + mAmbientState.getFractionToShade() + ); + } return mMaxAllowedKeyguardNotifications; } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewControllerTest.java index fa9161a19cb1a..f599e3b12c57e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewControllerTest.java @@ -577,20 +577,9 @@ public class NotificationPanelViewControllerTest extends SysuiTestCase { .isEqualTo(-1); } - @Test - public void computeMaxKeyguardNotifications_dozeAmountNotZero_returnsExistingMax() { - when(mAmbientState.getDozeAmount()).thenReturn(0.5f); - mNotificationPanelViewController.setMaxDisplayedNotifications(-1); - - // computeMaxKeyguardNotifications sets maxAllowed to 0 at minimum if it updates the value - assertThat(mNotificationPanelViewController.computeMaxKeyguardNotifications()) - .isEqualTo(-1); - } - @Test public void computeMaxKeyguardNotifications_noTransition_updatesMax() { when(mAmbientState.getFractionToShade()).thenReturn(0f); - when(mAmbientState.getDozeAmount()).thenReturn(0f); mNotificationPanelViewController.setMaxDisplayedNotifications(-1); // computeMaxKeyguardNotifications sets maxAllowed to 0 at minimum if it updates the value