diff options
7 files changed, 200 insertions, 82 deletions
diff --git a/packages/SystemUI/aconfig/systemui.aconfig b/packages/SystemUI/aconfig/systemui.aconfig index dace50fafbf1..a66ad19d5bd6 100644 --- a/packages/SystemUI/aconfig/systemui.aconfig +++ b/packages/SystemUI/aconfig/systemui.aconfig @@ -196,6 +196,18 @@ flag { } flag { + name: "notification_undo_guts_on_config_changed" + namespace: "systemui" + description: "Fixes a bug where a theme or font change while notification guts were open" + " (e.g. the snooze options or notification info) would show an empty notification by" + " closing the guts and undoing changes." + bug: "379267630" + metadata { + purpose: PURPOSE_BUGFIX + } +} + +flag { name: "pss_app_selector_recents_split_screen" namespace: "systemui" description: "Allows recent apps selected for partial screenshare to be launched in split screen mode" diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.kt index 39c42f183481..28b2ee8dde06 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.kt +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.kt @@ -269,6 +269,36 @@ class NotificationGutsManagerTest(flags: FlagsParameterization) : SysuiTestCase( } @Test + fun testOpenAndCloseGutsWithoutSave() { + val guts = spy(NotificationGuts(mContext)) + whenever(guts.post(any())).thenAnswer { invocation: InvocationOnMock -> + handler.post(((invocation.arguments[0] as Runnable))) + null + } + + // Test doesn't support animation since the guts view is not attached. + doNothing().whenever(guts).openControls(anyInt(), anyInt(), anyBoolean(), any()) + + val realRow = createTestNotificationRow() + val menuItem = createTestMenuItem(realRow) + + val row = spy(realRow) + whenever(row.windowToken).thenReturn(Binder()) + whenever(row.guts).thenReturn(guts) + + assertTrue(gutsManager.openGutsInternal(row, 0, 0, menuItem)) + executor.runAllReady() + verify(guts).openControls(anyInt(), anyInt(), anyBoolean(), any<Runnable>()) + + gutsManager.closeAndUndoGuts() + + verify(guts).closeControls(anyInt(), anyInt(), eq(false), eq(false)) + verify(row, times(1)).setGutsView(any<MenuItem>()) + executor.runAllReady() + verify(headsUpManager).setGutsShown(realRow.entry, false) + } + + @Test fun testLockscreenShadeVisible_visible_gutsNotClosed() = testScope.runTest { // First, start out lockscreen or shade as not visible @@ -377,52 +407,6 @@ class NotificationGutsManagerTest(flags: FlagsParameterization) : SysuiTestCase( } @Test - fun testChangeDensityOrFontScale() { - val guts = spy(NotificationGuts(mContext)) - whenever(guts.post(any())).thenAnswer { invocation: InvocationOnMock -> - handler.post(((invocation.arguments[0] as Runnable))) - null - } - - // Test doesn't support animation since the guts view is not attached. - doNothing().whenever(guts).openControls(anyInt(), anyInt(), anyBoolean(), any<Runnable>()) - - val realRow = createTestNotificationRow() - val menuItem = createTestMenuItem(realRow) - - val row = spy(realRow) - - whenever(row.windowToken).thenReturn(Binder()) - whenever(row.guts).thenReturn(guts) - doNothing().whenever(row).ensureGutsInflated() - - val realEntry = realRow.entry - val entry = spy(realEntry) - - whenever(entry.row).thenReturn(row) - whenever(entry.guts).thenReturn(guts) - - assertTrue(gutsManager.openGutsInternal(row, 0, 0, menuItem)) - executor.runAllReady() - verify(guts).openControls(anyInt(), anyInt(), anyBoolean(), any<Runnable>()) - - // called once by mGutsManager.bindGuts() in mGutsManager.openGuts() - verify(row).setGutsView(any<MenuItem>()) - - row.onDensityOrFontScaleChanged() - gutsManager.onDensityOrFontScaleChanged(entry) - - executor.runAllReady() - - gutsManager.closeAndSaveGuts(false, false, false, 0, 0, false) - - verify(guts).closeControls(anyBoolean(), anyBoolean(), anyInt(), anyInt(), anyBoolean()) - - // called again by mGutsManager.bindGuts(), in mGutsManager.onDensityOrFontScaleChanged() - verify(row, times(2)).setGutsView(any<MenuItem>()) - } - - @Test fun testAppOpsSettingsIntent_camera() { val row = createTestNotificationRow() val ops = ArraySet<Int>() diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationSnoozeTest.java b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationSnoozeTest.java index 6435e8203d3d..af67a04d2f2a 100644 --- a/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationSnoozeTest.java +++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationSnoozeTest.java @@ -16,29 +16,44 @@ package com.android.systemui.statusbar.notification.row; +import static com.google.common.truth.Truth.assertThat; + import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import android.platform.test.annotations.EnableFlags; import android.provider.Settings; import android.testing.TestableResources; -import android.util.KeyValueListParser; +import android.view.View; +import android.view.ViewGroup; +import android.widget.ImageView; +import android.widget.TextView; import androidx.test.annotation.UiThreadTest; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.SmallTest; +import com.android.systemui.Flags; import com.android.systemui.SysuiTestCase; +import com.android.systemui.animation.AnimatorTestRule; +import com.android.systemui.plugins.statusbar.NotificationSwipeActionHelper; import com.android.systemui.plugins.statusbar.NotificationSwipeActionHelper.SnoozeOption; import com.android.systemui.res.R; +import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import java.util.ArrayList; +import java.util.List; @SmallTest @RunWith(AndroidJUnit4.class) @@ -46,8 +61,12 @@ import java.util.ArrayList; public class NotificationSnoozeTest extends SysuiTestCase { private static final int RES_DEFAULT = 2; private static final int[] RES_OPTIONS = {1, 2, 3}; - private NotificationSnooze mNotificationSnooze; - private KeyValueListParser mMockParser; + private final NotificationSwipeActionHelper mSnoozeListener = mock( + NotificationSwipeActionHelper.class); + private NotificationSnooze mUnderTest; + + @Rule + public final AnimatorTestRule mAnimatorTestRule = new AnimatorTestRule(this); @Before public void setUp() throws Exception { @@ -56,62 +75,117 @@ public class NotificationSnoozeTest extends SysuiTestCase { TestableResources resources = mContext.getOrCreateTestableResources(); resources.addOverride(R.integer.config_notification_snooze_time_default, RES_DEFAULT); resources.addOverride(R.array.config_notification_snooze_times, RES_OPTIONS); - mNotificationSnooze = new NotificationSnooze(mContext, null); - mMockParser = mock(KeyValueListParser.class); + + mUnderTest = new NotificationSnooze(mContext, null); + mUnderTest.setSnoozeListener(mSnoozeListener); + mUnderTest.mExpandButton = mock(ImageView.class); + mUnderTest.mSnoozeView = mock(View.class); + mUnderTest.mSelectedOptionText = mock(TextView.class); + mUnderTest.mDivider = mock(View.class); + mUnderTest.mSnoozeOptionContainer = mock(ViewGroup.class); + mUnderTest.mSnoozeOptions = mock(List.class); + } + + @After + public void tearDown() { + // Make sure all animations are finished + mAnimatorTestRule.advanceTimeBy(1000L); + } + + @Test + @EnableFlags(Flags.FLAG_NOTIFICATION_UNDO_GUTS_ON_CONFIG_CHANGED) + public void closeControls_withoutSave_performsUndo() { + ArrayList<SnoozeOption> options = mUnderTest.getDefaultSnoozeOptions(); + mUnderTest.mSelectedOption = options.getFirst(); + mUnderTest.showSnoozeOptions(true); + + assertThat( + mUnderTest.handleCloseControls(/* save = */ false, /* force = */ false)).isFalse(); + + assertThat(mUnderTest.mSelectedOption).isNull(); + assertThat(mUnderTest.isExpanded()).isFalse(); + verify(mSnoozeListener, times(0)).snooze(any(), any()); + } + + @Test + public void closeControls_whenExpanded_collapsesOptions() { + ArrayList<SnoozeOption> options = mUnderTest.getDefaultSnoozeOptions(); + mUnderTest.mSelectedOption = options.getFirst(); + mUnderTest.showSnoozeOptions(true); + + assertThat(mUnderTest.handleCloseControls(/* save = */ true, /* force = */ false)).isTrue(); + + assertThat(mUnderTest.mSelectedOption).isNotNull(); + assertThat(mUnderTest.isExpanded()).isFalse(); + } + + @Test + public void closeControls_whenCollapsed_commitsChanges() { + ArrayList<SnoozeOption> options = mUnderTest.getDefaultSnoozeOptions(); + mUnderTest.mSelectedOption = options.getFirst(); + + assertThat(mUnderTest.handleCloseControls(/* save = */ true, /* force = */ false)).isTrue(); + + verify(mSnoozeListener).snooze(any(), any()); + } + + @Test + public void closeControls_withForce_returnsFalse() { + assertThat(mUnderTest.handleCloseControls(/* save = */ true, /* force = */ true)).isFalse(); } @Test - public void testGetOptionsWithNoConfig() throws Exception { - ArrayList<SnoozeOption> result = mNotificationSnooze.getDefaultSnoozeOptions(); + public void testGetOptionsWithNoConfig() { + ArrayList<SnoozeOption> result = mUnderTest.getDefaultSnoozeOptions(); assertEquals(3, result.size()); assertEquals(1, result.get(0).getMinutesToSnoozeFor()); // respect order assertEquals(2, result.get(1).getMinutesToSnoozeFor()); assertEquals(3, result.get(2).getMinutesToSnoozeFor()); - assertEquals(2, mNotificationSnooze.getDefaultOption().getMinutesToSnoozeFor()); + assertEquals(2, mUnderTest.getDefaultOption().getMinutesToSnoozeFor()); } @Test - public void testGetOptionsWithInvalidConfig() throws Exception { + public void testGetOptionsWithInvalidConfig() { Settings.Global.putString(mContext.getContentResolver(), Settings.Global.NOTIFICATION_SNOOZE_OPTIONS, "this is garbage"); - ArrayList<SnoozeOption> result = mNotificationSnooze.getDefaultSnoozeOptions(); + ArrayList<SnoozeOption> result = mUnderTest.getDefaultSnoozeOptions(); assertEquals(3, result.size()); assertEquals(1, result.get(0).getMinutesToSnoozeFor()); // respect order assertEquals(2, result.get(1).getMinutesToSnoozeFor()); assertEquals(3, result.get(2).getMinutesToSnoozeFor()); - assertEquals(2, mNotificationSnooze.getDefaultOption().getMinutesToSnoozeFor()); + assertEquals(2, mUnderTest.getDefaultOption().getMinutesToSnoozeFor()); } @Test - public void testGetOptionsWithValidDefault() throws Exception { + public void testGetOptionsWithValidDefault() { Settings.Global.putString(mContext.getContentResolver(), Settings.Global.NOTIFICATION_SNOOZE_OPTIONS, "default=10,options_array=4:5:6:7"); - ArrayList<SnoozeOption> result = mNotificationSnooze.getDefaultSnoozeOptions(); - assertNotNull(mNotificationSnooze.getDefaultOption()); // pick one + ArrayList<SnoozeOption> result = mUnderTest.getDefaultSnoozeOptions(); + assertNotNull(mUnderTest.getDefaultOption()); // pick one } @Test - public void testGetOptionsWithValidConfig() throws Exception { + public void testGetOptionsWithValidConfig() { Settings.Global.putString(mContext.getContentResolver(), Settings.Global.NOTIFICATION_SNOOZE_OPTIONS, "default=6,options_array=4:5:6:7"); - ArrayList<SnoozeOption> result = mNotificationSnooze.getDefaultSnoozeOptions(); + ArrayList<SnoozeOption> result = mUnderTest.getDefaultSnoozeOptions(); assertEquals(4, result.size()); assertEquals(4, result.get(0).getMinutesToSnoozeFor()); // respect order assertEquals(5, result.get(1).getMinutesToSnoozeFor()); assertEquals(6, result.get(2).getMinutesToSnoozeFor()); assertEquals(7, result.get(3).getMinutesToSnoozeFor()); - assertEquals(6, mNotificationSnooze.getDefaultOption().getMinutesToSnoozeFor()); + assertEquals(6, mUnderTest.getDefaultOption().getMinutesToSnoozeFor()); } @Test - public void testGetOptionsWithLongConfig() throws Exception { + public void testGetOptionsWithLongConfig() { Settings.Global.putString(mContext.getContentResolver(), Settings.Global.NOTIFICATION_SNOOZE_OPTIONS, "default=6,options_array=4:5:6:7:8:9:10:11:12:13:14:15:16:17"); - ArrayList<SnoozeOption> result = mNotificationSnooze.getDefaultSnoozeOptions(); + ArrayList<SnoozeOption> result = mUnderTest.getDefaultSnoozeOptions(); assertTrue(result.size() > 3); assertEquals(4, result.get(0).getMinutesToSnoozeFor()); // respect order assertEquals(5, result.get(1).getMinutesToSnoozeFor()); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt index 2d1eccdf1abd..a0a86710b4ba 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt @@ -22,6 +22,7 @@ import com.android.internal.widget.MessagingGroup import com.android.internal.widget.MessagingMessage import com.android.keyguard.KeyguardUpdateMonitor import com.android.keyguard.KeyguardUpdateMonitorCallback +import com.android.systemui.Flags import com.android.systemui.shade.ShadeDisplayAware import com.android.systemui.statusbar.NotificationLockscreenUserManager import com.android.systemui.statusbar.NotificationLockscreenUserManager.UserChangedListener @@ -144,7 +145,12 @@ internal constructor( ) log { "ViewConfigCoordinator.updateNotificationsOnUiModeChanged()" } traceSection("updateNotifOnUiModeChanged") { - mPipeline?.allNotifs?.forEach { entry -> entry.row?.onUiModeChanged() } + mPipeline?.allNotifs?.forEach { entry -> + entry.row?.onUiModeChanged() + if (Flags.notificationUndoGutsOnConfigChanged()) { + mGutsManager.closeAndUndoGuts() + } + } } } @@ -152,9 +158,15 @@ internal constructor( colorUpdateLogger.logEvent("VCC.updateNotificationsOnDensityOrFontScaleChanged()") mPipeline?.allNotifs?.forEach { entry -> entry.onDensityOrFontScaleChanged() - val exposedGuts = entry.areGutsExposed() - if (exposedGuts) { - mGutsManager.onDensityOrFontScaleChanged(entry) + if (Flags.notificationUndoGutsOnConfigChanged()) { + mGutsManager.closeAndUndoGuts() + } else { + // This property actually gets reset when the guts are re-inflated, so we're never + // actually calling onDensityOrFontScaleChanged below. + val exposedGuts = entry.areGutsExposed() + if (exposedGuts) { + mGutsManager.onDensityOrFontScaleChanged(entry) + } } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGuts.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGuts.java index b86d1d934269..75d1c7c3d51e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGuts.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGuts.java @@ -287,7 +287,7 @@ public class NotificationGuts extends FrameLayout { * @param save whether the state should be saved * @param force whether the guts should be force-closed regardless of state. */ - private void closeControls(int x, int y, boolean save, boolean force) { + public void closeControls(int x, int y, boolean save, boolean force) { // First try to dismiss any blocking helper. if (getWindowToken() == null) { if (mClosedListener != null) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java index b1e5b22f9b1a..445cd010cd86 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java @@ -48,6 +48,7 @@ import com.android.internal.logging.nano.MetricsProto; import com.android.internal.statusbar.IStatusBarService; import com.android.settingslib.notification.ConversationIconFactory; import com.android.systemui.CoreStartable; +import com.android.systemui.Flags; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dagger.qualifiers.Background; import com.android.systemui.dagger.qualifiers.Main; @@ -223,6 +224,10 @@ public class NotificationGutsManager implements NotifGutsViewManager, CoreStarta } public void onDensityOrFontScaleChanged(NotificationEntry entry) { + if (!Flags.notificationUndoGutsOnConfigChanged()) { + Log.wtf(TAG, "onDensityOrFontScaleChanged should not be called if" + + " notificationUndoGutsOnConfigChanged is off"); + } setExposedGuts(entry.getGuts()); bindGuts(entry.getRow()); } @@ -590,7 +595,8 @@ public class NotificationGutsManager implements NotifGutsViewManager, CoreStarta } /** - * Closes guts or notification menus that might be visible and saves any changes. + * Closes guts or notification menus that might be visible and saves any changes if applicable + * (see {@link NotificationGuts.GutsContent#shouldBeSavedOnClose}). * * @param removeLeavebehinds true if leavebehinds (e.g. snooze) should be closed. * @param force true if guts should be closed regardless of state (used for snooze only). @@ -611,6 +617,20 @@ public class NotificationGutsManager implements NotifGutsViewManager, CoreStarta } /** + * Closes all guts that might be visible without saving changes. + */ + public void closeAndUndoGuts() { + if (mNotificationGutsExposed != null) { + mNotificationGutsExposed.removeCallbacks(mOpenRunnable); + mNotificationGutsExposed.closeControls( + /* x = */ -1, + /* y = */ -1, + /* save = */ false, + /* force = */ false); + } + } + + /** * Returns the exposed NotificationGuts or null if none are exposed. */ public NotificationGuts getExposedGuts() { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationSnooze.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationSnooze.java index 99a6f6a59bd0..83897f5bc3a7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationSnooze.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationSnooze.java @@ -51,6 +51,7 @@ import com.android.app.animation.Interpolators; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; +import com.android.systemui.Flags; import com.android.systemui.plugins.statusbar.NotificationSwipeActionHelper; import com.android.systemui.plugins.statusbar.NotificationSwipeActionHelper.SnoozeOption; import com.android.systemui.res.R; @@ -86,18 +87,26 @@ public class NotificationSnooze extends LinearLayout private NotificationSwipeActionHelper mSnoozeListener; private StatusBarNotification mSbn; - private View mSnoozeView; - private TextView mSelectedOptionText; + @VisibleForTesting + public View mSnoozeView; + @VisibleForTesting + public TextView mSelectedOptionText; private TextView mUndoButton; - private ImageView mExpandButton; - private View mDivider; - private ViewGroup mSnoozeOptionContainer; - private List<SnoozeOption> mSnoozeOptions; + @VisibleForTesting + public ImageView mExpandButton; + @VisibleForTesting + public View mDivider; + @VisibleForTesting + public ViewGroup mSnoozeOptionContainer; + @VisibleForTesting + public List<SnoozeOption> mSnoozeOptions; private int mCollapsedHeight; private SnoozeOption mDefaultOption; - private SnoozeOption mSelectedOption; + @VisibleForTesting + public SnoozeOption mSelectedOption; private boolean mSnoozing; - private boolean mExpanded; + @VisibleForTesting + public boolean mExpanded; private AnimatorSet mExpandAnimation; private KeyValueListParser mParser; @@ -334,7 +343,8 @@ public class NotificationSnooze extends LinearLayout } } - private void showSnoozeOptions(boolean show) { + @VisibleForTesting + public void showSnoozeOptions(boolean show) { int drawableId = show ? com.android.internal.R.drawable.ic_collapse_notification : com.android.internal.R.drawable.ic_expand_notification; mExpandButton.setImageResource(drawableId); @@ -381,7 +391,8 @@ public class NotificationSnooze extends LinearLayout mExpandAnimation.start(); } - private void setSelected(SnoozeOption option, boolean userAction) { + @VisibleForTesting + public void setSelected(SnoozeOption option, boolean userAction) { if (option != mSelectedOption) { mSelectedOption = option; mSelectedOptionText.setText(option.getConfirmation()); @@ -466,7 +477,12 @@ public class NotificationSnooze extends LinearLayout @Override public boolean handleCloseControls(boolean save, boolean force) { - if (mExpanded && !force) { + if (Flags.notificationUndoGutsOnConfigChanged() && !save) { + // Undo changes and let the guts handle closing the view + mSelectedOption = null; + showSnoozeOptions(false); + return false; + } else if (mExpanded && !force) { // Collapse expanded state on outside touch showSnoozeOptions(false); return true; |