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

Commit aae95605 authored by Ioana Alexandru's avatar Ioana Alexandru Committed by Android (Google) Code Review
Browse files

Merge changes Ie7eef8c7,I6ef2411e,I50e9806a,I632daa93 into main

* changes:
  Implement hit-ratio dumping for NotifCollectionCache
  Introduce NotifCollectionCache
  Notif redesign: reduce conversation icon margin
  Notif redesign: Don't apply effects to app icons
parents 136a2353 d3f8fc56
Loading
Loading
Loading
Loading
+232 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2024 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.systemui.statusbar.notification.collection

import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.util.time.FakeSystemClock
import com.google.common.truth.Truth.assertThat
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import org.junit.After
import org.junit.Test
import org.junit.runner.RunWith

@SmallTest
@RunWith(AndroidJUnit4::class)
class NotifCollectionCacheTest : SysuiTestCase() {
    companion object {
        const val A = "a"
        const val B = "b"
        const val C = "c"
    }

    val systemClock = FakeSystemClock()
    val underTest =
        NotifCollectionCache<String>(purgeTimeoutMillis = 200L, systemClock = systemClock)

    @After
    fun cleanUp() {
        underTest.clear()
    }

    @Test
    fun fetch_isOnlyCalledOncePerEntry() {
        val fetchList = mutableListOf<String>()
        val fetch = { key: String ->
            fetchList.add(key)
            key
        }

        // Construct the cache and make sure fetch is called
        assertThat(underTest.getOrFetch(A, fetch)).isEqualTo(A)
        assertThat(underTest.getOrFetch(B, fetch)).isEqualTo(B)
        assertThat(underTest.getOrFetch(C, fetch)).isEqualTo(C)
        assertThat(fetchList).containsExactly(A, B, C).inOrder()

        // Verify that further calls don't trigger fetch again
        underTest.getOrFetch(A, fetch)
        underTest.getOrFetch(A, fetch)
        underTest.getOrFetch(B, fetch)
        underTest.getOrFetch(C, fetch)
        assertThat(fetchList).containsExactly(A, B, C).inOrder()

        // Verify that fetch gets called again if the entries are cleared
        underTest.clear()
        underTest.getOrFetch(A, fetch)
        assertThat(fetchList).containsExactly(A, B, C, A).inOrder()
    }

    @Test
    fun purge_beforeTimeout_doesNothing() {
        // Populate cache
        val fetch = { key: String -> key }
        underTest.getOrFetch(A, fetch)
        underTest.getOrFetch(B, fetch)
        underTest.getOrFetch(C, fetch)

        // B starts off with ♥ ︎♥︎
        assertThat(underTest.getLives(B)).isEqualTo(2)
        // First purge run removes a ︎♥︎
        underTest.purge(listOf(A, C))
        assertNotNull(underTest.cache[B])
        assertThat(underTest.getLives(B)).isEqualTo(1)
        // Second purge run done too early does nothing to B
        systemClock.advanceTime(100L)
        underTest.purge(listOf(A, C))
        assertNotNull(underTest.cache[B])
        assertThat(underTest.getLives(B)).isEqualTo(1)
        // Purge done after timeout (200ms) clears B
        systemClock.advanceTime(100L)
        underTest.purge(listOf(A, C))
        assertNull(underTest.cache[B])
    }

    @Test
    fun get_resetsLives() {
        // Populate cache
        val fetch = { key: String -> key }
        underTest.getOrFetch(A, fetch)
        underTest.getOrFetch(B, fetch)
        underTest.getOrFetch(C, fetch)

        // Bring B down to one ︎♥︎
        underTest.purge(listOf(A, C))
        assertThat(underTest.getLives(B)).isEqualTo(1)

        // Get should restore B to ♥ ︎♥︎
        underTest.getOrFetch(B, fetch)
        assertThat(underTest.getLives(B)).isEqualTo(2)

        // Subsequent purge should remove a life regardless of timing
        underTest.purge(listOf(A, C))
        assertThat(underTest.getLives(B)).isEqualTo(1)
    }

    @Test
    fun purge_resetsLives() {
        // Populate cache
        val fetch = { key: String -> key }
        underTest.getOrFetch(A, fetch)
        underTest.getOrFetch(B, fetch)
        underTest.getOrFetch(C, fetch)

        // Bring B down to one ︎♥︎
        underTest.purge(listOf(A, C))
        assertThat(underTest.getLives(B)).isEqualTo(1)

        // When B is back to wantedKeys, it is restored to to ♥ ︎♥ ︎︎
        underTest.purge(listOf(B))
        assertThat(underTest.getLives(B)).isEqualTo(2)
        assertThat(underTest.getLives(A)).isEqualTo(1)
        assertThat(underTest.getLives(C)).isEqualTo(1)

        // Subsequent purge should remove a life regardless of timing
        underTest.purge(listOf(A, C))
        assertThat(underTest.getLives(B)).isEqualTo(1)
    }

    @Test
    fun purge_worksWithMoreLives() {
        val multiLivesCache =
            NotifCollectionCache<String>(
                retainCount = 3,
                purgeTimeoutMillis = 100L,
                systemClock = systemClock,
            )

        // Populate cache
        val fetch = { key: String -> key }
        multiLivesCache.getOrFetch(A, fetch)
        multiLivesCache.getOrFetch(B, fetch)
        multiLivesCache.getOrFetch(C, fetch)

        // B starts off with ♥ ︎♥︎ ♥ ︎♥︎
        assertThat(multiLivesCache.getLives(B)).isEqualTo(4)
        // First purge run removes a ︎♥︎
        multiLivesCache.purge(listOf(A, C))
        assertNotNull(multiLivesCache.cache[B])
        assertThat(multiLivesCache.getLives(B)).isEqualTo(3)
        // Second purge run done too early does nothing to B
        multiLivesCache.purge(listOf(A, C))
        assertNotNull(multiLivesCache.cache[B])
        assertThat(multiLivesCache.getLives(B)).isEqualTo(3)
        // Staggered purge runs remove further ︎♥︎
        systemClock.advanceTime(100L)
        multiLivesCache.purge(listOf(A, C))
        assertNotNull(multiLivesCache.cache[B])
        assertThat(multiLivesCache.getLives(B)).isEqualTo(2)
        systemClock.advanceTime(100L)
        multiLivesCache.purge(listOf(A, C))
        assertNotNull(multiLivesCache.cache[B])
        assertThat(multiLivesCache.getLives(B)).isEqualTo(1)
        systemClock.advanceTime(100L)
        multiLivesCache.purge(listOf(A, C))
        assertNull(multiLivesCache.cache[B])
    }

    @Test
    fun purge_worksWithNoLives() {
        val noLivesCache =
            NotifCollectionCache<String>(
                retainCount = 0,
                purgeTimeoutMillis = 0L,
                systemClock = systemClock,
            )

        val fetch = { key: String -> key }
        noLivesCache.getOrFetch(A, fetch)
        noLivesCache.getOrFetch(B, fetch)
        noLivesCache.getOrFetch(C, fetch)

        // Purge immediately removes entry
        noLivesCache.purge(listOf(A, C))

        assertNotNull(noLivesCache.cache[A])
        assertNull(noLivesCache.cache[B])
        assertNotNull(noLivesCache.cache[C])
    }

    @Test
    fun hitsAndMisses_areAccurate() {
        val fetch = { key: String -> key }

        // Construct the cache
        assertThat(underTest.getOrFetch(A, fetch)).isEqualTo(A)
        assertThat(underTest.getOrFetch(B, fetch)).isEqualTo(B)
        assertThat(underTest.getOrFetch(C, fetch)).isEqualTo(C)
        assertThat(underTest.hits.get()).isEqualTo(0)
        assertThat(underTest.misses.get()).isEqualTo(3)

        // Verify that further calls count as hits
        underTest.getOrFetch(A, fetch)
        underTest.getOrFetch(A, fetch)
        underTest.getOrFetch(B, fetch)
        underTest.getOrFetch(C, fetch)
        assertThat(underTest.hits.get()).isEqualTo(4)
        assertThat(underTest.misses.get()).isEqualTo(3)

        // Verify that a miss is counted again if the entries are cleared
        underTest.clear()
        underTest.getOrFetch(A, fetch)
        assertThat(underTest.hits.get()).isEqualTo(4)
        assertThat(underTest.misses.get()).isEqualTo(4)
    }

    private fun <V> NotifCollectionCache<V>.getLives(key: String) = this.cache[key]?.lives
}
+3 −1
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import com.android.internal.widget.ConversationLayout
import com.android.internal.widget.MessagingGroup
import com.android.internal.widget.MessagingImageMessage
import com.android.internal.widget.MessagingLinearLayout
import com.android.internal.widget.NotificationRowIconView
import com.android.systemui.SysuiTestCase
import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow
import com.android.systemui.statusbar.notification.row.NotificationTestHelper
@@ -90,7 +91,7 @@ class NotificationConversationTemplateViewWrapperTest : SysuiTestCase() {

    private fun fakeConversationLayout(
        mockDrawableGroupMessage: AnimatedImageDrawable,
        mockDrawableImageMessage: AnimatedImageDrawable
        mockDrawableImageMessage: AnimatedImageDrawable,
    ): View {
        val mockMessagingImageMessage: MessagingImageMessage =
            mock<MessagingImageMessage>().apply {
@@ -126,6 +127,7 @@ class NotificationConversationTemplateViewWrapperTest : SysuiTestCase() {
                whenever(requireViewById<CachingIconView>(R.id.conversation_icon))
                    .thenReturn(mock())
                whenever(findViewById<CachingIconView>(R.id.icon)).thenReturn(mock())
                whenever(requireViewById<NotificationRowIconView>(R.id.icon)).thenReturn(mock())
                whenever(requireViewById<View>(R.id.conversation_icon_badge_bg)).thenReturn(mock())
                whenever(requireViewById<View>(R.id.expand_button)).thenReturn(mock())
                whenever(requireViewById<View>(R.id.expand_button_container)).thenReturn(mock())
+39 −18
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@

package com.android.systemui.statusbar;

import android.app.Flags;
import android.app.Notification;
import android.graphics.drawable.Drawable;
import android.graphics.drawable.Icon;
@@ -60,20 +61,6 @@ public class NotificationGroupingUtil {
            return row.getEntry().getSbn().getNotification();
        }
    };
    private static final IconComparator ICON_VISIBILITY_COMPARATOR = new IconComparator() {
        public boolean compare(View parent, View child, Object parentData,
                Object childData) {
            return hasSameIcon(parentData, childData)
                    && hasSameColor(parentData, childData);
        }
    };
    private static final IconComparator GREY_COMPARATOR = new IconComparator() {
        public boolean compare(View parent, View child, Object parentData,
                Object childData) {
            return !hasSameIcon(parentData, childData)
                    || hasSameColor(parentData, childData);
        }
    };
    private static final ResultApplicator GREY_APPLICATOR = new ResultApplicator() {
        @Override
        public void apply(View parent, View view, boolean apply, boolean reset) {
@@ -90,34 +77,58 @@ public class NotificationGroupingUtil {

    public NotificationGroupingUtil(ExpandableNotificationRow row) {
        mRow = row;

        final IconComparator iconVisibilityComparator = new IconComparator(mRow) {
            public boolean compare(View parent, View child, Object parentData,
                    Object childData) {
                return hasSameIcon(parentData, childData)
                        && hasSameColor(parentData, childData);
            }
        };
        final IconComparator greyComparator = new IconComparator(mRow) {
            public boolean compare(View parent, View child, Object parentData,
                    Object childData) {
                if (Flags.notificationsRedesignAppIcons() && mRow.isShowingAppIcon()) {
                    return false;
                }
                return !hasSameIcon(parentData, childData)
                        || hasSameColor(parentData, childData);
            }
        };

        // To hide the icons if they are the same and the color is the same
        mProcessors.add(new Processor(mRow,
                com.android.internal.R.id.icon,
                ICON_EXTRACTOR,
                ICON_VISIBILITY_COMPARATOR,
                iconVisibilityComparator,
                VISIBILITY_APPLICATOR));
        // To grey them out the icons and expand button when the icons are not the same
        // To grey out the icons when they are not the same, or they have the same color
        mProcessors.add(new Processor(mRow,
                com.android.internal.R.id.status_bar_latest_event_content,
                ICON_EXTRACTOR,
                GREY_COMPARATOR,
                greyComparator,
                GREY_APPLICATOR));
        // To show the large icon on the left side instead if all the small icons are the same
        mProcessors.add(new Processor(mRow,
                com.android.internal.R.id.status_bar_latest_event_content,
                ICON_EXTRACTOR,
                ICON_VISIBILITY_COMPARATOR,
                iconVisibilityComparator,
                LEFT_ICON_APPLICATOR));
        // To only show the work profile icon in the group header
        mProcessors.add(new Processor(mRow,
                com.android.internal.R.id.profile_badge,
                null /* Extractor */,
                BADGE_COMPARATOR,
                VISIBILITY_APPLICATOR));
        // To hide the app name in group children
        mProcessors.add(new Processor(mRow,
                com.android.internal.R.id.app_name_text,
                null,
                APP_NAME_COMPARATOR,
                APP_NAME_APPLICATOR));
        // To hide the header text if it's the same
        mProcessors.add(Processor.forTextView(mRow, com.android.internal.R.id.header_text));

        mDividers.add(com.android.internal.R.id.header_text_divider);
        mDividers.add(com.android.internal.R.id.header_text_secondary_divider);
        mDividers.add(com.android.internal.R.id.time_divider);
@@ -261,6 +272,7 @@ public class NotificationGroupingUtil {
            mParentData = mExtractor == null ? null : mExtractor.extractData(mParentRow);
            mApply = !mComparator.isEmpty(mParentView);
        }

        public void compareToGroupParent(ExpandableNotificationRow row) {
            if (!mApply) {
                return;
@@ -356,12 +368,21 @@ public class NotificationGroupingUtil {
    }

    private abstract static class IconComparator implements ViewComparator {
        private final ExpandableNotificationRow mRow;

        IconComparator(ExpandableNotificationRow row) {
            mRow = row;
        }

        @Override
        public boolean compare(View parent, View child, Object parentData, Object childData) {
            return false;
        }

        protected boolean hasSameIcon(Object parentData, Object childData) {
            if (Flags.notificationsRedesignAppIcons() && mRow.isShowingAppIcon()) {
                return true;
            }
            Icon parentIcon = getIcon((Notification) parentData);
            Icon childIcon = getIcon((Notification) childData);
            return parentIcon.sameAs(childIcon);
+188 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2024 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.systemui.statusbar.notification.collection

import android.annotation.SuppressLint
import com.android.internal.annotations.VisibleForTesting
import com.android.systemui.Dumpable
import com.android.systemui.util.asIndenting
import com.android.systemui.util.printCollection
import com.android.systemui.util.time.SystemClock
import com.android.systemui.util.time.SystemClockImpl
import com.android.systemui.util.withIncreasedIndent
import java.io.PrintWriter
import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.atomic.AtomicInteger

/**
 * A cache in which entries can "survive" getting purged [retainCount] times, given consecutive
 * [purge] calls made at least [purgeTimeoutMillis] apart. See also [purge].
 *
 * This cache is safe for multithreaded usage, and is recommended for objects that take a while to
 * resolve (such as drawables, or things that require binder calls). As such, [getOrFetch] is
 * recommended to be run on a background thread, while [purge] can be done from any thread.
 */
@SuppressLint("DumpableNotRegistered") // this will be dumped by container classes
class NotifCollectionCache<V>(
    private val retainCount: Int = 1,
    private val purgeTimeoutMillis: Long = 1000L,
    private val systemClock: SystemClock = SystemClockImpl(),
) : Dumpable {
    @get:VisibleForTesting val cache = ConcurrentHashMap<String, CacheEntry>()

    // Counters for cache hits and misses to be used to calculate and dump the hit ratio
    @get:VisibleForTesting val misses = AtomicInteger(0)
    @get:VisibleForTesting val hits = AtomicInteger(0)

    init {
        if (retainCount < 0) {
            throw IllegalArgumentException("retainCount cannot be negative")
        }
    }

    inner class CacheEntry(val key: String, val value: V) {
        /**
         * The "lives" represent how many times the entry will remain in the cache when purging it
         * is attempted.
         */
        @get:VisibleForTesting var lives: Int = retainCount + 1
        /**
         * The last time this entry lost a "life". Starts at a negative value chosen so that the
         * first purge is always considered "valid".
         */
        private var lastValidPurge: Long = -purgeTimeoutMillis

        fun resetLives() {
            // Lives/timeouts don't matter if retainCount is 0
            if (retainCount == 0) {
                return
            }

            synchronized(key) {
                lives = retainCount + 1
                lastValidPurge = -purgeTimeoutMillis
            }
            // Add it to the cache again just in case it was deleted before we could reset the lives
            cache[key] = this
        }

        fun tryPurge(): Boolean {
            // Lives/timeouts don't matter if retainCount is 0
            if (retainCount == 0) {
                return true
            }

            // Using uptimeMillis since it's guaranteed to be monotonic, as we don't want a
            // timezone/clock change to break us
            val now = systemClock.uptimeMillis()

            // Cannot purge the same entry from two threads simultaneously
            synchronized(key) {
                if (now - lastValidPurge < purgeTimeoutMillis) {
                    return false
                }
                lastValidPurge = now
                return --lives <= 0
            }
        }
    }

    /**
     * Get value from cache, or fetch it and add it to cache if not found. This can be called from
     * any thread, but is usually expected to be called from the background.
     *
     * @param key key for the object to be obtained
     * @param fetch method to fetch the object and add it to the cache if not present; note that
     *   there is no guarantee that two [fetch] cannot run in parallel for the same [key] (if
     *   [getOrFetch] is called simultaneously from different threads), so be mindful of potential
     *   side effects
     */
    fun getOrFetch(key: String, fetch: (String) -> V): V {
        val entry = cache[key]
        if (entry != null) {
            hits.incrementAndGet()
            // Refresh lives on access
            entry.resetLives()
            return entry.value
        }

        misses.incrementAndGet()
        val value = fetch(key)
        cache[key] = CacheEntry(key, value)
        return value
    }

    /**
     * Clear entries that are NOT in [wantedKeys] if appropriate. This can be called from any
     * thread.
     *
     * If retainCount > 0, a given entry will need to not be present in [wantedKeys] for
     * ([retainCount] + 1) consecutive [purge] calls made within at least [purgeTimeoutMillis] of
     * each other in order to be cleared. This count will be reset for any given entry 1) if
     * [getOrFetch] is called for the entry or 2) if the entry is present in [wantedKeys] in a
     * subsequent [purge] call. We prioritize keeping the entry if possible, so if [purge] is called
     * simultaneously with [getOrFetch] on different threads for example, we will try to keep it in
     * the cache, although it is not guaranteed. If avoiding cache misses is a concern, consider
     * increasing the [retainCount] or [purgeTimeoutMillis].
     *
     * For example, say [retainCount] = 1 and [purgeTimeoutMillis] = 1000 and we start with entries
     * (a, b, c) in the cache:
     * ```kotlin
     * purge((a, c)); // marks b for deletion
     * Thread.sleep(500)
     * purge((a, c)); // does nothing as it was called earlier than the min 1s
     * Thread.sleep(500)
     * purge((b, c)); // b is no longer marked for deletion, but now a is
     * Thread.sleep(1000);
     * purge((c));    // deletes a from the cache and marks b for deletion, etc.
     * ```
     */
    fun purge(wantedKeys: List<String>) {
        for ((key, entry) in cache) {
            if (key in wantedKeys) {
                entry.resetLives()
            } else if (entry.tryPurge()) {
                cache.remove(key)
            }
        }
    }

    /** Clear all entries from the cache. */
    fun clear() {
        cache.clear()
    }

    override fun dump(pwOrig: PrintWriter, args: Array<out String>) {
        val pw = pwOrig.asIndenting()

        pw.println("$TAG(retainCount = $retainCount, purgeTimeoutMillis = $purgeTimeoutMillis)")
        pw.withIncreasedIndent {
            pw.printCollection("keys present in cache", cache.keys.stream().sorted().toList())

            val misses = misses.get()
            val hits = hits.get()
            pw.println(
                "cache hit ratio = ${(hits.toFloat() / (hits + misses)) * 100}% " +
                    "($hits hits, $misses misses)"
            )
        }
    }

    companion object {
        const val TAG = "NotifCollectionCache"
    }
}
+19 −0
Original line number Diff line number Diff line
@@ -262,6 +262,11 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
     */
    private boolean mIsHeadsUp;

    /**
     * Whether or not the notification is showing the app icon instead of the small icon.
     */
    private boolean mIsShowingAppIcon;

    private boolean mLastChronometerRunning = true;
    private ViewStub mChildrenContainerStub;
    private GroupMembershipManager mGroupMembershipManager;
@@ -816,6 +821,20 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
        }
    }

    /**
     * Indicate that the notification is showing the app icon instead of the small icon.
     */
    public void setIsShowingAppIcon(boolean isShowingAppIcon) {
        mIsShowingAppIcon = isShowingAppIcon;
    }

    /**
     * Whether or not the notification is showing the app icon instead of the small icon.
     */
    public boolean isShowingAppIcon() {
        return mIsShowingAppIcon;
    }

    @Override
    public boolean showingPulsing() {
        return isHeadsUpState() && (isDozing() || (mOnKeyguard && isBypassEnabled()));
Loading