From 38a76cc8e646ae50b7394171f0b53c2d203b184e Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Tue, 31 May 2022 16:40:16 -0400 Subject: [PATCH] Fix the lockscreen height recalculation on face auth. When face auth happens, the SensitiveContentCoordinator calls setSensitive on every visible notification, which triggers the views to suddenly show their private version. This often has a new height, and that new height affects both the height of the stack and how many notifications are on screen. Often things would get fixed once the ambinent indication appeared, because that was an input to the calculation, but if your phone had another ambient indication already on when your face triggered the unlock (e.g. charging) then that trigger didn't work. This CL ensures that when setSensitive() is called, calls notifyHeightChanged (when a height change is detected). This ensures that the NSSL's existing listener recalculates the stack height. This fixes the case where a single redacted notification would be truncated after unlock. This CL also ensures that when notifyHeightChanged() is called, the NotificationPanelView calls updateMaxDisplayedNotifications (when on the keyguard). This fixes the much more rare case where the number of notifications shown would be incorrect until some other change triggered the recalculation. Fixes: 230016435 Test: significant manual testing Test: atest NotificationPanelViewControllerTest ExpandableNotificationRowTest Change-Id: I509f97afef16a723ba43dbd58ae589c961d78755 --- .../row/ExpandableNotificationRow.java | 5 + .../NotificationPanelViewController.java | 3 + .../row/ExpandableNotificationRowTest.java | 99 +++++++++++++++++++ .../row/NotificationTestHelper.java | 15 ++- .../NotificationPanelViewControllerTest.java | 35 +++++++ 5 files changed, 152 insertions(+), 5 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java index 1cae60b3d37dd..e1caafff95456 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java @@ -2588,8 +2588,13 @@ public class ExpandableNotificationRow extends ActivatableNotificationView } public void setSensitive(boolean sensitive, boolean hideSensitive) { + int intrinsicBefore = getIntrinsicHeight(); mSensitive = sensitive; mSensitiveHiddenInGeneral = hideSensitive; + if (intrinsicBefore != getIntrinsicHeight()) { + // The animation has a few flaws and is highly visible, so jump cut instead. + notifyHeightChanged(false /* needsAnimation */); + } } @Override 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 9afdfd6511307..c521161e85f2b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java @@ -4362,6 +4362,9 @@ public class NotificationPanelViewController extends PanelViewController { == firstRow))) { requestScrollerTopPaddingUpdate(false /* animate */); } + if (mKeyguardShowing) { + updateMaxDisplayedNotifications(true); + } requestPanelHeightUpdate(); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java index b0da3eb84802e..c36737c6724ee 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java @@ -24,6 +24,8 @@ import static com.android.systemui.statusbar.NotificationEntryHelper.modifyRanki import static com.android.systemui.statusbar.NotificationEntryHelper.modifySbn; import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_ALL; +import static com.google.common.truth.Truth.assertThat; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -39,10 +41,12 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.app.Notification; import android.app.NotificationChannel; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.testing.TestableLooper.RunWithLooper; +import android.util.DisplayMetrics; import android.view.View; import androidx.test.filters.SmallTest; @@ -53,6 +57,7 @@ import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.notification.AboveShelfChangedListener; import com.android.systemui.statusbar.notification.FeedbackIcon; +import com.android.systemui.statusbar.notification.row.ExpandableView.OnHeightChangedListener; import com.android.systemui.statusbar.notification.stack.NotificationChildrenContainer; import org.junit.Assert; @@ -71,6 +76,8 @@ import java.util.List; public class ExpandableNotificationRowTest extends SysuiTestCase { private ExpandableNotificationRow mGroupRow; + private ExpandableNotificationRow mNotifRow; + private ExpandableNotificationRow mPublicRow; private NotificationTestHelper mNotificationTestHelper; boolean mHeadsUpAnimatingAway = false; @@ -84,9 +91,101 @@ public class ExpandableNotificationRowTest extends SysuiTestCase { mContext, mDependency, TestableLooper.get(this)); + mNotificationTestHelper.setDefaultInflationFlags(FLAG_CONTENT_VIEW_ALL); + // create a standard private notification row + Notification normalNotif = mNotificationTestHelper.createNotification(); + normalNotif.publicVersion = null; + mNotifRow = mNotificationTestHelper.createRow(normalNotif); + // create a notification row whose public version is identical + Notification publicNotif = mNotificationTestHelper.createNotification(); + publicNotif.publicVersion = mNotificationTestHelper.createNotification(); + mPublicRow = mNotificationTestHelper.createRow(publicNotif); + // create a group row mGroupRow = mNotificationTestHelper.createGroup(); mGroupRow.setHeadsUpAnimatingAwayListener( animatingAway -> mHeadsUpAnimatingAway = animatingAway); + + } + + @Test + public void testSetSensitiveOnNotifRowNotifiesOfHeightChange() throws InterruptedException { + // GIVEN a sensitive notification row that's currently redacted + measureAndLayout(mNotifRow); + mNotifRow.setHideSensitiveForIntrinsicHeight(true); + mNotifRow.setSensitive(true, true); + assertThat(mNotifRow.getShowingLayout()).isSameInstanceAs(mNotifRow.getPublicLayout()); + assertThat(mNotifRow.getIntrinsicHeight()).isGreaterThan(0); + + // GIVEN that the row has a height change listener + OnHeightChangedListener listener = mock(OnHeightChangedListener.class); + mNotifRow.setOnHeightChangedListener(listener); + + // WHEN the row is set to no longer be sensitive + mNotifRow.setSensitive(false, true); + + // VERIFY that the height change listener is invoked + assertThat(mNotifRow.getShowingLayout()).isSameInstanceAs(mNotifRow.getPrivateLayout()); + assertThat(mNotifRow.getIntrinsicHeight()).isGreaterThan(0); + verify(listener).onHeightChanged(eq(mNotifRow), eq(false)); + } + + @Test + public void testSetSensitiveOnGroupRowNotifiesOfHeightChange() { + // GIVEN a sensitive group row that's currently redacted + measureAndLayout(mGroupRow); + mGroupRow.setHideSensitiveForIntrinsicHeight(true); + mGroupRow.setSensitive(true, true); + assertThat(mGroupRow.getShowingLayout()).isSameInstanceAs(mGroupRow.getPublicLayout()); + assertThat(mGroupRow.getIntrinsicHeight()).isGreaterThan(0); + + // GIVEN that the row has a height change listener + OnHeightChangedListener listener = mock(OnHeightChangedListener.class); + mGroupRow.setOnHeightChangedListener(listener); + + // WHEN the row is set to no longer be sensitive + mGroupRow.setSensitive(false, true); + + // VERIFY that the height change listener is invoked + assertThat(mGroupRow.getShowingLayout()).isSameInstanceAs(mGroupRow.getPrivateLayout()); + assertThat(mGroupRow.getIntrinsicHeight()).isGreaterThan(0); + verify(listener).onHeightChanged(eq(mGroupRow), eq(false)); + } + + @Test + public void testSetSensitiveOnPublicRowDoesNotNotifyOfHeightChange() { + // GIVEN a sensitive public row that's currently redacted + measureAndLayout(mPublicRow); + mPublicRow.setHideSensitiveForIntrinsicHeight(true); + mPublicRow.setSensitive(true, true); + assertThat(mPublicRow.getShowingLayout()).isSameInstanceAs(mPublicRow.getPublicLayout()); + assertThat(mPublicRow.getIntrinsicHeight()).isGreaterThan(0); + + // GIVEN that the row has a height change listener + OnHeightChangedListener listener = mock(OnHeightChangedListener.class); + mPublicRow.setOnHeightChangedListener(listener); + + // WHEN the row is set to no longer be sensitive + mPublicRow.setSensitive(false, true); + + // VERIFY that the height change listener is not invoked, because the height didn't change + assertThat(mPublicRow.getShowingLayout()).isSameInstanceAs(mPublicRow.getPrivateLayout()); + assertThat(mPublicRow.getIntrinsicHeight()).isGreaterThan(0); + assertThat(mPublicRow.getPrivateLayout().getMinHeight()) + .isEqualTo(mPublicRow.getPublicLayout().getMinHeight()); + verify(listener, never()).onHeightChanged(eq(mPublicRow), eq(false)); + } + + private void measureAndLayout(ExpandableNotificationRow row) { + DisplayMetrics dm = new DisplayMetrics(); + getContext().getDisplay().getRealMetrics(dm); + int width = (int) Math.ceil(400f * dm.density); + int height = (int) Math.ceil(600f * dm.density); + + row.measure( + View.MeasureSpec.makeMeasureSpec(width, View.MeasureSpec.EXACTLY), + View.MeasureSpec.makeMeasureSpec(height, View.MeasureSpec.UNSPECIFIED) + ); + row.layout(0, 0, row.getMeasuredWidth(), row.getMeasuredHeight()); } @Test diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java index eceff7cc63623..38bd078589dc3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationTestHelper.java @@ -124,6 +124,7 @@ public class NotificationTestHelper { private final PeopleNotificationIdentifier mPeopleNotificationIdentifier; public final OnUserInteractionCallback mOnUserInteractionCallback; public final Runnable mFutureDismissalRunnable; + private @InflationFlag int mDefaultInflationFlags; public NotificationTestHelper( Context context, @@ -189,6 +190,10 @@ public class NotificationTestHelper { .thenReturn(mFutureDismissalRunnable); } + public void setDefaultInflationFlags(@InflationFlag int defaultInflationFlags) { + mDefaultInflationFlags = defaultInflationFlags; + } + /** * Creates a generic row. * @@ -220,7 +225,7 @@ public class NotificationTestHelper { * @throws Exception */ public ExpandableNotificationRow createRow(Notification notification) throws Exception { - return generateRow(notification, PKG, UID, USER_HANDLE, 0 /* extraInflationFlags */); + return generateRow(notification, PKG, UID, USER_HANDLE, mDefaultInflationFlags); } /** @@ -271,7 +276,7 @@ public class NotificationTestHelper { null /* groupKey */, makeBubbleMetadata(null)); n.flags |= FLAG_BUBBLE; ExpandableNotificationRow row = generateRow(n, PKG, UID, USER_HANDLE, - 0 /* extraInflationFlags */, IMPORTANCE_HIGH); + mDefaultInflationFlags, IMPORTANCE_HIGH); modifyRanking(row.getEntry()) .setCanBubble(true) .build(); @@ -287,7 +292,7 @@ public class NotificationTestHelper { null /* groupKey */, makeShortcutBubbleMetadata(shortcutId)); n.flags |= FLAG_BUBBLE; ExpandableNotificationRow row = generateRow(n, PKG, UID, USER_HANDLE, - 0 /* extraInflationFlags */, IMPORTANCE_HIGH); + mDefaultInflationFlags, IMPORTANCE_HIGH); modifyRanking(row.getEntry()) .setCanBubble(true) .build(); @@ -304,7 +309,7 @@ public class NotificationTestHelper { GROUP_KEY /* groupKey */, makeBubbleMetadata(null)); n.flags |= FLAG_BUBBLE; ExpandableNotificationRow row = generateRow(n, PKG, UID, USER_HANDLE, - 0 /* extraInflationFlags */, IMPORTANCE_HIGH); + mDefaultInflationFlags, IMPORTANCE_HIGH); modifyRanking(row.getEntry()) .setCanBubble(true) .build(); @@ -383,7 +388,7 @@ public class NotificationTestHelper { @Nullable String groupKey) throws Exception { Notification notif = createNotification(isGroupSummary, groupKey); - return generateRow(notif, pkg, uid, userHandle, 0 /* inflationFlags */); + return generateRow(notif, pkg, uid, userHandle, mDefaultInflationFlags); } /** 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 f599e3b12c57e..62058a179dac7 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 @@ -28,6 +28,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyFloat; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeast; @@ -121,6 +122,8 @@ import com.android.systemui.statusbar.notification.ConversationNotificationManag import com.android.systemui.statusbar.notification.DynamicPrivacyController; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.NotificationWakeUpCoordinator; +import com.android.systemui.statusbar.notification.row.ExpandableView; +import com.android.systemui.statusbar.notification.row.ExpandableView.OnHeightChangedListener; import com.android.systemui.statusbar.notification.stack.AmbientState; import com.android.systemui.statusbar.notification.stack.NotificationListContainer; import com.android.systemui.statusbar.notification.stack.NotificationRoundnessManager; @@ -567,6 +570,38 @@ public class NotificationPanelViewControllerTest extends SysuiTestCase { mMainHandler.removeCallbacksAndMessages(null); } + @Test + public void onNotificationHeightChangeWhileOnKeyguardWillComputeMaxKeyguardNotifications() { + mStatusBarStateController.setState(KEYGUARD); + ArgumentCaptor captor = + ArgumentCaptor.forClass(OnHeightChangedListener.class); + verify(mNotificationStackScrollLayoutController) + .setOnHeightChangedListener(captor.capture()); + OnHeightChangedListener listener = captor.getValue(); + + clearInvocations(mNotificationStackSizeCalculator); + listener.onHeightChanged(mock(ExpandableView.class), false); + + verify(mNotificationStackSizeCalculator) + .computeMaxKeyguardNotifications(any(), anyFloat(), anyFloat()); + } + + @Test + public void onNotificationHeightChangeWhileInShadeWillNotComputeMaxKeyguardNotifications() { + mStatusBarStateController.setState(SHADE); + ArgumentCaptor captor = + ArgumentCaptor.forClass(OnHeightChangedListener.class); + verify(mNotificationStackScrollLayoutController) + .setOnHeightChangedListener(captor.capture()); + OnHeightChangedListener listener = captor.getValue(); + + clearInvocations(mNotificationStackSizeCalculator); + listener.onHeightChanged(mock(ExpandableView.class), false); + + verify(mNotificationStackSizeCalculator, never()) + .computeMaxKeyguardNotifications(any(), anyFloat(), anyFloat()); + } + @Test public void computeMaxKeyguardNotifications_lockscreenToShade_returnsExistingMax() { when(mAmbientState.getFractionToShade()).thenReturn(0.5f);