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

Commit 497e7df5 authored by Ioana Alexandru's avatar Ioana Alexandru
Browse files

Close and undo guts on configuration changed 

If a guts view like snooze options or notification info was exposed when
the theme or the font changed, it would result in a seemingly empty
notification.

Although we had an onDensityOrFontScaleChanged that was supposed to at
least re-bind the guts when the font was changed, this actually did
nothing because when the guts were re-inflated, the "exposed" state of
the guts was getting reset (so this method was never called).

Now we just undo the changes and close the guts, to make sure they look
correct the next time they are opened.

Bug: 379267630
Test: snooze notification and change the theme via QS
Test: long press notification and change theme/font via adb
Test: atest NotificationSnoozeTest NotificationGutsManagerTest
Flag: com.android.systemui.notification_undo_guts_on_config_changed
Change-Id: I7e3caaf8895a3756bb9f4ad4f627cd66d44a8748
parent 53d05392

packages/SystemUI/aconfig/systemui.aconfig

+12 −0
Original line number Diff line number Diff line
@@ -195,6 +195,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"

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.kt

+30 −46
Original line number Diff line number Diff line
@@ -268,6 +268,36 @@ class NotificationGutsManagerTest(flags: FlagsParameterization) : SysuiTestCase( 
        verify(headsUpManager).setGutsShown(realRow.entry, false)

    }



    @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 {
@@ -376,52 +406,6 @@ class NotificationGutsManagerTest(flags: FlagsParameterization) : SysuiTestCase( 
            verify(notificationListContainer).resetExposedMenuView(anyBoolean(), anyBoolean())

        }



    @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()

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/row/NotificationSnoozeTest.java

+93 −19
Original line number Diff line number Diff line
@@ -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());

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/ViewConfigCoordinator.kt

+16 −4
Original line number Diff line number Diff line
@@ -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,12 +158,18 @@ internal constructor( 
        colorUpdateLogger.logEvent("VCC.updateNotificationsOnDensityOrFontScaleChanged()")

        mPipeline?.allNotifs?.forEach { entry ->

            entry.onDensityOrFontScaleChanged()

            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)

                }

            }

        }

    }



    private inline fun log(message: () -> String) {

        if (DEBUG) Log.d(TAG, message())

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGuts.java

+1 −1
Original line number Diff line number Diff line
@@ -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) {