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

Commit 594551f4 authored by Jieru Shi's avatar Jieru Shi
Browse files

Media impression logging bug fix 

1. Remove redundant LS impressions: Bouncer->LS, Shade LS->LS (User has to visit LS first in order to visit Bouncer and Shade LS)
2. Remove extra impression when user is on second media card, tap to app
or turn off screen (previously there will be an impression logged for
first media card)
3. Fix logging for resumption media card, previously they were not
logged.
4. Fix impression logging for reactivated media card when user connect headphone on
QQS, previously they were not logged
5. Use SmallHash to compute instanceid according to b/190640624
6. Remove media resume card logging when Smartspace data is not
available

Bug: 181364757
Test: manual
Change-Id: I6e2e7bc00ecd3f21fefedb2c47315b1e85e5beeb
parent 60ec24eb

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

+67 −12
Original line number Diff line number Diff line
@@ -156,6 +156,12 @@ class MediaCarouselController @Inject constructor( 
        }

    }



    /**

     * Update MediaCarouselScrollHandler.visibleToUser to reflect media card container visibility.

     * It will be called when the container is out of view.

     */

    lateinit var updateUserVisibility: () -> Unit



    init {

        mediaFrame = inflateMediaCarousel()

        mediaCarousel = mediaFrame.requireViewById(R.id.media_carousel_scroller)
@@ -177,6 +183,12 @@ class MediaCarouselController @Inject constructor( 
            keysNeedRemoval.forEach { removePlayer(it) }

            keysNeedRemoval.clear()



            // Update user visibility so that no extra impression will be logged when

            // activeMediaIndex resets to 0

            if (this::updateUserVisibility.isInitialized) {

                updateUserVisibility()

            }



            // Let's reset our scroll position

            mediaCarouselScrollHandler.scrollToStart()

        }
@@ -187,16 +199,24 @@ class MediaCarouselController @Inject constructor( 
                key: String,

                oldKey: String?,

                data: MediaData,

                immediately: Boolean

                immediately: Boolean,

                isSsReactivated: Boolean

            ) {

                if (addOrUpdatePlayer(key, oldKey, data)) {

                    MediaPlayerData.getMediaPlayer(key, null)?.let {

                        logSmartspaceCardReported(759, // SMARTSPACE_CARD_RECEIVED

                                it.mInstanceId,

                                /* isRecommendationCard */ false,

                                it.surfaceForSmartspaceLogging)

                                it.surfaceForSmartspaceLogging,

                                rank = MediaPlayerData.getMediaPlayerIndex(key))

                    }

                }

                if (mediaCarouselScrollHandler.visibleToUser &&

                        isSsReactivated && !mediaCarouselScrollHandler.qsExpanded) {

                    // It could happen that reactived media player isn't visible to user because

                    // of it is a resumption card.

                    logSmartspaceImpression(mediaCarouselScrollHandler.qsExpanded)

                }

                val canRemove = data.isPlaying?.let { !it } ?: data.isClearable && !data.active

                if (canRemove && !Utils.useMediaResumption(context)) {

                    // This view isn't playing, let's remove this! This happens e.g when
@@ -222,12 +242,19 @@ class MediaCarouselController @Inject constructor( 
                    addSmartspaceMediaRecommendations(key, data, shouldPrioritize)

                    MediaPlayerData.getMediaPlayer(key, null)?.let {

                        logSmartspaceCardReported(759, // SMARTSPACE_CARD_RECEIVED

                                it.mInstanceId,

                                /* isRecommendationCard */ true,

                                it.surfaceForSmartspaceLogging,

                                rank = MediaPlayerData.getMediaPlayerIndex(key))



                        if (mediaCarouselScrollHandler.visibleToUser &&

                                mediaCarouselScrollHandler.visibleMediaIndex ==

                                MediaPlayerData.getMediaPlayerIndex(key)) {

                            logSmartspaceCardReported(800, // SMARTSPACE_CARD_SEEN

                                    it.mInstanceId,

                                    /* isRecommendationCard */ true,

                                    it.surfaceForSmartspaceLogging)

                        }

                    if (mediaCarouselScrollHandler.visibleToUser) {

                        logSmartspaceImpression()

                    }

                } else {

                    onSmartspaceMediaDataRemoved(data.targetId, immediately = true)
@@ -644,17 +671,17 @@ class MediaCarouselController @Inject constructor( 
    }



    /**

     * Log the user impression for media card.

     * Log the user impression for media card at visibleMediaIndex.

     */

    fun logSmartspaceImpression() {

    fun logSmartspaceImpression(qsExpanded: Boolean) {

        val visibleMediaIndex = mediaCarouselScrollHandler.visibleMediaIndex

        if (MediaPlayerData.players().size > visibleMediaIndex) {

            val mediaControlPanel = MediaPlayerData.players().elementAt(visibleMediaIndex)

            val isMediaActive =

                    MediaPlayerData.playerKeys().elementAt(visibleMediaIndex).data?.active

            val hasActiveMediaOrRecommendationCard =

                    MediaPlayerData.hasActiveMediaOrRecommendationCard()

            val isRecommendationCard = mediaControlPanel.recommendationViewHolder != null

            if (!isRecommendationCard && !isMediaActive) {

                // Media control card time out or swiped away

            if (!hasActiveMediaOrRecommendationCard && !qsExpanded) {

                // Skip logging if on LS or QQS, and there is no active media card

                return

            }

            logSmartspaceCardReported(800, // SMARTSPACE_CARD_SEEN
@@ -672,6 +699,13 @@ class MediaCarouselController @Inject constructor( 
        surface: Int,

        rank: Int = mediaCarouselScrollHandler.visibleMediaIndex

    ) {

        // Only log media resume card when Smartspace data is available

        if (!isRecommendationCard &&

                        !mediaManager.smartspaceMediaData.isActive &&

                                MediaPlayerData.smartspaceMediaData == null) {

            return

        }



        /* ktlint-disable max-line-length */

        SysUiStatsLog.write(SysUiStatsLog.SMARTSPACE_CARD_REPORTED,

                eventId,
@@ -770,6 +804,16 @@ internal object MediaPlayerData { 
        return mediaData.get(key)?.let { mediaPlayers.get(it) }

    }



    fun getMediaPlayerIndex(key: String): Int {

        val sortKey = mediaData.get(key)

        mediaPlayers.entries.forEachIndexed { index, e ->

            if (e.key == sortKey) {

                return index

            }

        }

        return -1

    }



    fun removeMediaPlayer(key: String) = mediaData.remove(key)?.let {

        if (it.isSsMediaRec) {

            smartspaceMediaData = null
@@ -808,4 +852,15 @@ internal object MediaPlayerData { 
        mediaData.clear()

        mediaPlayers.clear()

    }



    /* Returns true if there is active media player card or recommendation card */

    fun hasActiveMediaOrRecommendationCard(): Boolean {

        if (smartspaceMediaData != null && smartspaceMediaData?.isActive!!) {

            return true

        }

        if (firstActiveMediaIndex() != -1) {

            return true

        }

        return false

    }

}

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

+11 −7
Original line number Diff line number Diff line
@@ -62,7 +62,7 @@ class MediaCarouselScrollHandler( 
    private val closeGuts: (immediate: Boolean) -> Unit,

    private val falsingCollector: FalsingCollector,

    private val falsingManager: FalsingManager,

    private val logSmartspaceImpression: () -> Unit

    private val logSmartspaceImpression: (Boolean) -> Unit

) {

    /**

     * Is the view in RTL
@@ -195,18 +195,22 @@ class MediaCarouselScrollHandler( 
            if (playerWidthPlusPadding == 0) {

                return

            }



            val relativeScrollX = scrollView.relativeScrollX

            onMediaScrollingChanged(relativeScrollX / playerWidthPlusPadding,

                    relativeScrollX % playerWidthPlusPadding)

        }

    }



    /**

     * Whether the media card is visible to user if any

     */

    var visibleToUser: Boolean = false

        set(value) {

            if (field != value) {

                field = value

            }

        }



    /**

     * Whether the quick setting is expanded or not

     */

    var qsExpanded: Boolean = false



    init {

        gestureDetector = GestureDetectorCompat(scrollView.context, gestureListener)
@@ -471,7 +475,7 @@ class MediaCarouselScrollHandler( 
            val oldIndex = visibleMediaIndex

            visibleMediaIndex = newIndex

            if (oldIndex != visibleMediaIndex && visibleToUser) {

                logSmartspaceImpression()

                logSmartspaceImpression(qsExpanded)

            }

            closeGuts(false)

            updatePlayerVisibilities()

packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java

+2 −2
Original line number Diff line number Diff line
@@ -266,7 +266,7 @@ public class MediaControlPanel { 
        }

        mKey = key;

        MediaSession.Token token = data.getToken();

        mInstanceId = data.getPackageName().hashCode();

        mInstanceId = SmallHash.hash(data.getPackageName());



        mBackgroundColor = data.getBackgroundColor();

        if (mToken == null || !mToken.equals(token)) {
@@ -504,7 +504,7 @@ public class MediaControlPanel { 
            return;

        }



        mInstanceId = data.getTargetId().hashCode();

        mInstanceId = SmallHash.hash(data.getTargetId());

        mBackgroundColor = data.getBackgroundColor();

        TransitionLayout recommendationCard = mRecommendationViewHolder.getRecommendations();

        recommendationCard.setBackgroundTintList(ColorStateList.valueOf(mBackgroundColor));

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

+2 −1
Original line number Diff line number Diff line
@@ -31,7 +31,8 @@ class MediaDataCombineLatest @Inject constructor() : MediaDataManager.Listener, 
        key: String,

        oldKey: String?,

        data: MediaData,

        immediately: Boolean

        immediately: Boolean,

        isSsReactivated: Boolean

    ) {

        if (oldKey != null && oldKey != key && entries.contains(oldKey)) {

            entries[key] = data to entries.remove(oldKey)?.second

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

+10 −3
Original line number Diff line number Diff line
@@ -83,7 +83,8 @@ class MediaDataFilter @Inject constructor( 
        key: String,

        oldKey: String?,

        data: MediaData,

        immediately: Boolean

        immediately: Boolean,

        isSsReactivated: Boolean

    ) {

        if (oldKey != null && oldKey != key) {

            allEntries.remove(oldKey)
@@ -101,7 +102,7 @@ class MediaDataFilter @Inject constructor( 


        // Notify listeners

        listeners.forEach {

            it.onMediaDataLoaded(key, oldKey, data)

            it.onMediaDataLoaded(key, oldKey, data, isSsReactivated = isSsReactivated)

        }

    }
@@ -118,6 +119,8 @@ class MediaDataFilter @Inject constructor( 
        // Override the pass-in value here, as the order of Smartspace card is only determined here.

        var shouldPrioritizeMutable = false

        smartspaceMediaData = data

        // Override the pass-in value here, as the Smartspace reactivation could only happen here.

        var isSsReactivated = false



        // Before forwarding the smartspace target, first check if we have recently inactive media

        val sorted = userEntries.toSortedMap(compareBy {
@@ -137,9 +140,13 @@ class MediaDataFilter @Inject constructor( 
            // Notify listeners to consider this media active

            Log.d(TAG, "reactivating $lastActiveKey instead of smartspace")

            reactivatedKey = lastActiveKey

            if (MediaPlayerData.firstActiveMediaIndex() == -1) {

                isSsReactivated = true

            }

            val mediaData = sorted.get(lastActiveKey)!!.copy(active = true)

            listeners.forEach {

                it.onMediaDataLoaded(lastActiveKey, lastActiveKey, mediaData)

                it.onMediaDataLoaded(lastActiveKey, lastActiveKey, mediaData,

                        isSsReactivated = isSsReactivated)

            }

        } else {

            // Mark to prioritize Smartspace card if no recent media.