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

Commit 2f601c76 authored by lyn's avatar lyn
Browse files

Fix classified children showing as heads up 

This issue happened because summaries
- rarely have text
- will rarely be classified
- can still transfer heads up behavior to silent classified children

In HeadsUpCoordinator's onBeforeFinalizeFilter,
do not transfer alerts to classified children,
which are silent and should not heads up.

Fixes: 401101330
Test: HeadsUpCoordinatorTest
Flag: android.service.notification.notification_classification
Change-Id: If6cffe2b21d62c0832efac375cc1ebb26af029f8
parent e50dd973

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

+47 −1
Original line number Diff line number Diff line
@@ -17,7 +17,9 @@ package com.android.systemui.statusbar.notification.collection.coordinator 


import android.app.Notification.GROUP_ALERT_ALL

import android.app.Notification.GROUP_ALERT_SUMMARY

import android.app.NotificationChannel.SYSTEM_RESERVED_IDS

import android.platform.test.annotations.EnableFlags

import android.service.notification.Flags.FLAG_NOTIFICATION_CLASSIFICATION

import android.testing.TestableLooper.RunWithLooper

import androidx.test.ext.junit.runners.AndroidJUnit4

import androidx.test.filters.SmallTest
@@ -49,6 +51,7 @@ import com.android.systemui.statusbar.notification.interruption.HeadsUpViewBinde 
import com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProvider.FullScreenIntentDecision

import com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProviderWrapper.DecisionImpl

import com.android.systemui.statusbar.notification.interruption.NotificationInterruptStateProviderWrapper.FullScreenIntentDecisionImpl

import com.android.systemui.statusbar.notification.interruption.VisualInterruptionDecisionLogger

import com.android.systemui.statusbar.notification.interruption.VisualInterruptionDecisionProvider

import com.android.systemui.statusbar.notification.row.mockNotificationActionClickManager

import com.android.systemui.statusbar.notification.shared.NotificationBundleUi
@@ -60,7 +63,6 @@ import com.android.systemui.util.mockito.eq 
import com.android.systemui.util.mockito.mock

import com.android.systemui.util.mockito.withArgCaptor

import com.android.systemui.util.time.FakeSystemClock

import java.util.ArrayList

import java.util.function.Consumer

import kotlinx.coroutines.test.runTest

import org.junit.Assert.assertEquals
@@ -104,6 +106,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() { 


    private val notifPipeline: NotifPipeline = mock()

    private val logger = HeadsUpCoordinatorLogger(logcatLogBuffer(), verbose = true)

    private val interruptLogger: VisualInterruptionDecisionLogger = mock()

    private val headsUpManager: HeadsUpManagerImpl = mock()

    private val headsUpViewBinder: HeadsUpViewBinder = mock()

    private val visualInterruptionDecisionProvider: VisualInterruptionDecisionProvider = mock()
@@ -134,6 +137,7 @@ class HeadsUpCoordinatorTest : SysuiTestCase() { 
            HeadsUpCoordinator(

                kosmos.applicationCoroutineScope,

                logger,

                interruptLogger,

                systemClock,

                notifCollection,

                headsUpManager,
@@ -918,6 +922,48 @@ class HeadsUpCoordinatorTest : SysuiTestCase() { 
        assertFalse(groupSummary.hasInterrupted())

    }



    private fun helpTestNoTransferToBundleChildForChannel(channelId: String) {

        // Set up for normal alert transfer from summary to child

        // but here child is classified so it should not happen

        val bundleChild =

            helper.createClassifiedEntry(/* isSummary= */ false, GROUP_ALERT_SUMMARY, channelId);

        setShouldHeadsUp(bundleChild, true)

        setShouldHeadsUp(groupSummary, true)

        whenever(notifPipeline.allNotifs).thenReturn(listOf(groupSummary, bundleChild))



        collectionListener.onEntryAdded(groupSummary)

        collectionListener.onEntryAdded(bundleChild)



        beforeTransformGroupsListener.onBeforeTransformGroups(listOf(groupSummary, bundleChild))

        beforeFinalizeFilterListener.onBeforeFinalizeFilter(listOf(groupSummary, bundleChild))



        verify(headsUpViewBinder, never()).bindHeadsUpView(eq(groupSummary), any(), any())

        verify(headsUpViewBinder, never()).bindHeadsUpView(eq(bundleChild), any(), any())



        verify(headsUpManager, never()).showNotification(groupSummary)

        verify(headsUpManager, never()).showNotification(bundleChild)



        // Capture last param

        val decision = withArgCaptor {

            verify(interruptLogger)

                .logDecision(capture(), capture(), capture())

        }

        assertFalse(decision.shouldInterrupt)

        assertEquals(decision.logReason, "disqualified-transfer-target")



        // Must clear invocations, otherwise these calls get stored for the next call from the same

        // test, which complains that there are more invocations than expected

        clearInvocations(interruptLogger)

    }



    @Test

    @EnableFlags(FLAG_NOTIFICATION_CLASSIFICATION)

    fun testNoTransfer_toBundleChild() {

        for (id in SYSTEM_RESERVED_IDS) {

            helpTestNoTransferToBundleChildForChannel(id)

        }

    }



    @Test

    fun testOnRankingApplied_newEntryShouldAlert() {

        // GIVEN that mEntry has never interrupted in the past, and now should

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/phone/NotificationGroupTestHelper.java

+29 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


package com.android.systemui.statusbar.phone;



import static android.app.NotificationManager.IMPORTANCE_LOW;

import static com.google.common.truth.Truth.assertThat;



import static org.mockito.Mockito.mock;
@@ -23,6 +24,7 @@ import static org.mockito.Mockito.when; 


import android.app.ActivityManager;

import android.app.Notification;

import android.app.NotificationChannel;

import android.content.Context;

import android.os.UserHandle;

import android.service.notification.StatusBarNotification;
@@ -85,6 +87,33 @@ public final class NotificationGroupTestHelper { 
        return entry;

    }



    public NotificationEntry createClassifiedEntry(boolean isSummary,

            int groupAlertBehavior, String channelId) {



        Notification notif = new Notification.Builder(mContext, TEST_CHANNEL_ID)

                .setContentTitle("Title")

                .setSmallIcon(R.drawable.ic_person)

                .setGroupAlertBehavior(groupAlertBehavior)

                .setGroupSummary(isSummary)

                .setGroup(TEST_GROUP_ID)

                .build();



        NotificationChannel channel = new NotificationChannel(channelId, channelId, IMPORTANCE_LOW);

        NotificationEntry entry = new NotificationEntryBuilder()

                .setPkg(TEST_PACKAGE_NAME)

                .setOpPkg(TEST_PACKAGE_NAME)

                .setId(mId++)

                .setNotification(notif)

                .updateRanking((rankingBuilder -> rankingBuilder.setChannel(channel)))

                .setUser(new UserHandle(ActivityManager.getCurrentUser()))

                .build();



        ExpandableNotificationRow row = mock(ExpandableNotificationRow.class);

        entry.setRow(row);

        when(row.getEntryLegacy()).thenReturn(entry);

        return entry;

    }



    public NotificationEntry createEntry(int id, String tag, boolean isSummary,

            int groupAlertBehavior) {

        Notification notif = new Notification.Builder(mContext, TEST_CHANNEL_ID)

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

+26 −0
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ import com.android.systemui.Flags.notificationSkipSilentUpdates 


import android.app.Notification

import android.app.Notification.GROUP_ALERT_SUMMARY

import android.app.NotificationChannel.SYSTEM_RESERVED_IDS

import android.util.ArrayMap

import android.util.ArraySet

import com.android.internal.annotations.VisibleForTesting
@@ -48,7 +49,10 @@ import com.android.systemui.statusbar.notification.headsup.HeadsUpManager 
import com.android.systemui.statusbar.notification.headsup.OnHeadsUpChangedListener

import com.android.systemui.statusbar.notification.headsup.PinnedStatus

import com.android.systemui.statusbar.notification.interruption.HeadsUpViewBinder

import com.android.systemui.statusbar.notification.interruption.VisualInterruptionDecisionLogger

import com.android.systemui.statusbar.notification.interruption.VisualInterruptionDecisionProvider

import com.android.systemui.statusbar.notification.interruption.VisualInterruptionDecisionProviderImpl.DecisionImpl

import com.android.systemui.statusbar.notification.interruption.VisualInterruptionType

import com.android.systemui.statusbar.notification.logKey

import com.android.systemui.statusbar.notification.row.NotificationActionClickManager

import com.android.systemui.statusbar.notification.shared.GroupHunAnimationFix
@@ -81,6 +85,7 @@ class HeadsUpCoordinator 
constructor(

    @Application private val applicationScope: CoroutineScope,

    private val mLogger: HeadsUpCoordinatorLogger,

    private val mInterruptLogger: VisualInterruptionDecisionLogger,

    private val mSystemClock: SystemClock,

    private val notifCollection: NotifCollection,

    private val mHeadsUpManager: HeadsUpManager,
@@ -281,6 +286,19 @@ constructor( 
                    return@forEach

                }



                if (isDisqualifiedChild(childToReceiveParentHeadsUp)) {

                    mInterruptLogger.logDecision(

                        VisualInterruptionType.PEEK.name,

                        childToReceiveParentHeadsUp,

                        DecisionImpl(shouldInterrupt = false,

                            logReason = "disqualified-transfer-target"))

                    postedEntries.forEach {

                        it.shouldHeadsUpEver = false

                        it.shouldHeadsUpAgain = false

                        handlePostedEntry(it, hunMutator, scenario = "disqualified-transfer-target")

                    }

                    return@forEach

                }

                // At this point we just need to initiate the transfer

                val summaryUpdate = mPostedEntries[logicalSummary.key]
@@ -383,6 +401,14 @@ constructor( 
            cleanUpEntryTimes()

        }



    private fun isDisqualifiedChild(entry: NotificationEntry): Boolean  {

        if (entry.channel == null || entry.channel.id == null) {

            return false

        }

        return entry.channel.id in SYSTEM_RESERVED_IDS

    }





    /**

     * Find the posted child with the newest when, and return it if it is isolated and has

     * GROUP_ALERT_SUMMARY so that it can be heads uped.

packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/VisualInterruptionDecisionProviderImpl.kt

+1 −1
Original line number Diff line number Diff line
@@ -87,7 +87,7 @@ constructor( 
        val eventLogData: EventLogData?

    }



    private class DecisionImpl(

    class DecisionImpl(

        override val shouldInterrupt: Boolean,

        override val logReason: String,

    ) : Decision