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

Commit f291b434 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Automerger Merge Worker
Browse files

Merge "[Media] Add logs for potential memory leaks in... 

Merge "[Media] Add logs for potential memory leaks in MediaCarouselController." into tm-dev am: eccd6122

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/18320596



Change-Id: Id45df4a4905ade47cc27db9358ee9c1541590057
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents f15a8c30 eccd6122

packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java

+12 −0
Original line number Diff line number Diff line
@@ -230,6 +230,18 @@ public class LogModule { 
        return factory.create("MediaBrowser", 100);

    }



    /**

     * Provides a buffer for updates to the media carousel.

     *

     * See {@link com.android.systemui.media.MediaCarouselController}.

     */

    @Provides

    @SysUISingleton

    @MediaCarouselControllerLog

    public static LogBuffer provideMediaCarouselControllerBuffer(LogBufferFactory factory) {

        return factory.create("MediaCarouselCtlrLog", 20);

    }



    /** Allows logging buffers to be tweaked via adb on debug builds but not on prod builds. */

    @Provides

    @SysUISingleton

packages/SystemUI/src/com/android/systemui/log/dagger/MediaCarouselControllerLog.java

0 → 100644
+35 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2022 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.log.dagger;



import static java.lang.annotation.RetentionPolicy.RUNTIME;



import com.android.systemui.log.LogBuffer;



import java.lang.annotation.Documented;

import java.lang.annotation.Retention;



import javax.inject.Qualifier;



/**

 * A {@link LogBuffer} for {@link com.android.systemui.media.MediaCarouselController}

 */

@Qualifier

@Documented

@Retention(RUNTIME)

public @interface MediaCarouselControllerLog {

}

packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt

+36 −12
Original line number Diff line number Diff line
@@ -59,7 +59,8 @@ class MediaCarouselController @Inject constructor( 
    falsingCollector: FalsingCollector,

    falsingManager: FalsingManager,

    dumpManager: DumpManager,

    private val logger: MediaUiEventLogger

    private val logger: MediaUiEventLogger,

    private val debugLogger: MediaCarouselControllerLogger

) : Dumpable {

    /**

     * The current width of the carousel
@@ -439,12 +440,16 @@ class MediaCarouselController @Inject constructor( 
            newPlayer.mediaViewHolder?.player?.setLayoutParams(lp)

            newPlayer.bindPlayer(data, key)

            newPlayer.setListening(currentlyExpanded)

            MediaPlayerData.addMediaPlayer(key, data, newPlayer, systemClock, isSsReactivated)

            MediaPlayerData.addMediaPlayer(

                key, data, newPlayer, systemClock, isSsReactivated, debugLogger

            )

            updatePlayerToState(newPlayer, noAnimation = true)

            reorderAllPlayers(curVisibleMediaKey)

        } else {

            existingPlayer.bindPlayer(data, key)

            MediaPlayerData.addMediaPlayer(key, data, existingPlayer, systemClock, isSsReactivated)

            MediaPlayerData.addMediaPlayer(

                key, data, existingPlayer, systemClock, isSsReactivated, debugLogger

            )

            if (isReorderingAllowed || shouldScrollToActivePlayer) {

                reorderAllPlayers(curVisibleMediaKey)

            } else {
@@ -475,7 +480,8 @@ class MediaCarouselController @Inject constructor( 


        val existingSmartspaceMediaKey = MediaPlayerData.smartspaceMediaKey()

        existingSmartspaceMediaKey?.let {

            MediaPlayerData.removeMediaPlayer(existingSmartspaceMediaKey)

            val removedPlayer = MediaPlayerData.removeMediaPlayer(existingSmartspaceMediaKey)

            removedPlayer?.run { debugLogger.logPotentialMemoryLeak(existingSmartspaceMediaKey) }

        }



        val newRecs = mediaControlPanelFactory.get()
@@ -488,7 +494,9 @@ class MediaCarouselController @Inject constructor( 
        newRecs.bindRecommendation(data)

        val curVisibleMediaKey = MediaPlayerData.playerKeys()

                .elementAtOrNull(mediaCarouselScrollHandler.visibleMediaIndex)

        MediaPlayerData.addMediaRecommendation(key, data, newRecs, shouldPrioritize, systemClock)

        MediaPlayerData.addMediaRecommendation(

            key, data, newRecs, shouldPrioritize, systemClock, debugLogger

        )

        updatePlayerToState(newRecs, noAnimation = true)

        reorderAllPlayers(curVisibleMediaKey)

        updatePageIndicator()
@@ -883,7 +891,8 @@ class MediaCarouselController @Inject constructor( 
    override fun dump(pw: PrintWriter, args: Array<out String>) {

        pw.apply {

            println("keysNeedRemoval: $keysNeedRemoval")

            println("playerKeys: ${MediaPlayerData.playerKeys()}")

            println("dataKeys: ${MediaPlayerData.dataKeys()}")

            println("playerSortKeys: ${MediaPlayerData.playerKeys()}")

            println("smartspaceMediaData: ${MediaPlayerData.smartspaceMediaData}")

            println("shouldPrioritizeSs: ${MediaPlayerData.shouldPrioritizeSs}")

            println("current size: $currentCarouselWidth x $currentCarouselHeight")
@@ -946,9 +955,13 @@ internal object MediaPlayerData { 
        data: MediaData,

        player: MediaControlPanel,

        clock: SystemClock,

        isSsReactivated: Boolean

        isSsReactivated: Boolean,

        debugLogger: MediaCarouselControllerLogger? = null

    ) {

        removeMediaPlayer(key)

        val removedPlayer = removeMediaPlayer(key)

        if (removedPlayer != null && removedPlayer != player) {

            debugLogger?.logPotentialMemoryLeak(key)

        }

        val sortKey = MediaSortKey(isSsMediaRec = false,

                data, clock.currentTimeMillis(), isSsReactivated = isSsReactivated)

        mediaData.put(key, sortKey)
@@ -960,10 +973,14 @@ internal object MediaPlayerData { 
        data: SmartspaceMediaData,

        player: MediaControlPanel,

        shouldPrioritize: Boolean,

        clock: SystemClock

        clock: SystemClock,

        debugLogger: MediaCarouselControllerLogger? = null

    ) {

        shouldPrioritizeSs = shouldPrioritize

        removeMediaPlayer(key)

        val removedPlayer = removeMediaPlayer(key)

        if (removedPlayer != null && removedPlayer != player) {

            debugLogger?.logPotentialMemoryLeak(key)

        }

        val sortKey = MediaSortKey(isSsMediaRec = true,

            EMPTY.copy(isPlaying = false), clock.currentTimeMillis(), isSsReactivated = true)

        mediaData.put(key, sortKey)
@@ -971,13 +988,18 @@ internal object MediaPlayerData { 
        smartspaceMediaData = data

    }



    fun moveIfExists(oldKey: String?, newKey: String) {

    fun moveIfExists(

        oldKey: String?,

        newKey: String,

        debugLogger: MediaCarouselControllerLogger? = null

    ) {

        if (oldKey == null || oldKey == newKey) {

            return

        }



        mediaData.remove(oldKey)?.let {

            removeMediaPlayer(newKey)

            val removedPlayer = removeMediaPlayer(newKey)

            removedPlayer?.run { debugLogger?.logPotentialMemoryLeak(newKey) }

            mediaData.put(newKey, it)

        }

    }
@@ -1005,6 +1027,8 @@ internal object MediaPlayerData { 


    fun mediaData() = mediaData.entries.map { e -> Triple(e.key, e.value.data, e.value.isSsMediaRec) }



    fun dataKeys() = mediaData.keys



    fun players() = mediaPlayers.values



    fun playerKeys() = mediaPlayers.keys

packages/SystemUI/src/com/android/systemui/media/MediaCarouselControllerLogger.kt

0 → 100644
+45 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2022 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.media



import com.android.systemui.dagger.SysUISingleton

import com.android.systemui.log.LogBuffer

import com.android.systemui.log.LogLevel

import com.android.systemui.log.dagger.MediaCarouselControllerLog

import javax.inject.Inject



/** A debug logger for [MediaCarouselController]. */

@SysUISingleton

class MediaCarouselControllerLogger @Inject constructor(

    @MediaCarouselControllerLog private val buffer: LogBuffer

) {

    /**

     * Log that there might be a potential memory leak for the [MediaControlPanel] and/or

     * [MediaViewController] related to [key].

     */

    fun logPotentialMemoryLeak(key: String) = buffer.log(

        TAG,

        LogLevel.DEBUG,

        { str1 = key },

        {

            "Potential memory leak: " +

                    "Removing control panel for $str1 from map without calling #onDestroy"

        }

    )

}



private const val TAG = "MediaCarouselCtlrLog"

packages/SystemUI/tests/src/com/android/systemui/media/MediaCarouselControllerTest.kt

+3 −1
Original line number Diff line number Diff line
@@ -63,6 +63,7 @@ class MediaCarouselControllerTest : SysuiTestCase() { 
    @Mock lateinit var falsingManager: FalsingManager

    @Mock lateinit var dumpManager: DumpManager

    @Mock lateinit var logger: MediaUiEventLogger

    @Mock lateinit var debugLogger: MediaCarouselControllerLogger



    private val clock = FakeSystemClock()

    private lateinit var mediaCarouselController: MediaCarouselController
@@ -83,7 +84,8 @@ class MediaCarouselControllerTest : SysuiTestCase() { 
            falsingCollector,

            falsingManager,

            dumpManager,

            logger

            logger,

            debugLogger

        )



        MediaPlayerData.clear()