From d0a83fe892edf2149868910c54026643e0226848 Mon Sep 17 00:00:00 2001 From: Beth Thibodeau Date: Wed, 10 Nov 2021 21:30:26 -0500 Subject: Sort remote cast notifications to the end Remote cast notifications are posted by gmscore when the screen turns on. Since they are not user-initiated, and since lastUpdated won't be meaningful, sort them behind any other active media controls in the carousel. To avoid hard coding packages, we determine whether the notification is an RCN by whether it is from a privileged app + sets a substitute app name, which is only done by RCN currently - as noted in the comment we plan to add an explicit API for this in the T release Fixes: 204935142 Test: atest com.android.systemui.media Test: manual Change-Id: Ia5d1e1f544b546644941f05efda199844aae74e5 --- .../systemui/media/MediaCarouselController.kt | 15 ++++---- .../src/com/android/systemui/media/MediaData.kt | 19 ++++++++-- .../com/android/systemui/media/MediaDataManager.kt | 33 ++++++++++++++--- .../android/systemui/media/MediaResumeListener.kt | 2 +- .../systemui/media/MediaCarouselControllerTest.kt | 41 +++++++++++++++------- .../systemui/media/MediaDataCombineLatestTest.java | 4 +-- .../android/systemui/media/MediaDataManagerTest.kt | 28 ++++++++++++++- .../android/systemui/media/MediaPlayerDataTest.kt | 20 ++++++----- .../systemui/media/MediaResumeListenerTest.kt | 18 +++++++--- 9 files changed, 137 insertions(+), 43 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt index e87558ebee27..47ef5e4c62fd 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt @@ -833,12 +833,15 @@ internal object MediaPlayerData { ) private val comparator = - compareByDescending { it.data.isPlaying == true && it.data.isLocalSession } - .thenByDescending { it.data.isPlaying } - .thenByDescending { if (shouldPrioritizeSs) it.isSsMediaRec else !it.isSsMediaRec } - .thenByDescending { !it.data.resumption } - .thenByDescending { it.updateTime } - .thenByDescending { !it.data.isLocalSession } + compareByDescending { it.data.isPlaying == true && + it.data.playbackLocation == MediaData.PLAYBACK_LOCAL } + .thenByDescending { it.data.isPlaying == true && + it.data.playbackLocation == MediaData.PLAYBACK_CAST_LOCAL } + .thenByDescending { if (shouldPrioritizeSs) it.isSsMediaRec else !it.isSsMediaRec } + .thenByDescending { !it.data.resumption } + .thenByDescending { it.data.playbackLocation != MediaData.PLAYBACK_CAST_REMOTE } + .thenByDescending { it.updateTime } + .thenByDescending { it.data.notificationKey } private val mediaPlayers = TreeMap(comparator) private val mediaData: MutableMap = mutableMapOf() diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaData.kt b/packages/SystemUI/src/com/android/systemui/media/MediaData.kt index 6f0c88710461..bda07f428218 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaData.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaData.kt @@ -82,9 +82,9 @@ data class MediaData( */ var resumeAction: Runnable?, /** - * Local or remote playback + * Playback location: one of PLAYBACK_LOCAL, PLAYBACK_CAST_LOCAL, or PLAYBACK_CAST_REMOTE */ - var isLocalSession: Boolean = true, + var playbackLocation: Int = PLAYBACK_LOCAL, /** * Indicates that this player is a resumption player (ie. It only shows a play actions which * will start the app and start playing). @@ -110,7 +110,20 @@ data class MediaData( * Timestamp when this player was last active. */ var lastActive: Long = 0L -) +) { + companion object { + /** Media is playing on the local device */ + const val PLAYBACK_LOCAL = 0 + /** Media is cast but originated on the local device */ + const val PLAYBACK_CAST_LOCAL = 1 + /** Media is from a remote cast notification */ + const val PLAYBACK_CAST_REMOTE = 2 + } + + fun isLocalSession(): Boolean { + return playbackLocation == PLAYBACK_LOCAL + } +} /** State of a media action. */ data class MediaAction( diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt index 3631d2fa04fb..21b548f06e01 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt @@ -27,6 +27,8 @@ import android.content.ContentResolver import android.content.Context import android.content.Intent import android.content.IntentFilter +import android.content.pm.ApplicationInfo +import android.content.pm.PackageManager import android.graphics.Bitmap import android.graphics.Canvas import android.graphics.ImageDecoder @@ -153,6 +155,24 @@ class MediaDataManager( private var smartspaceSession: SmartspaceSession? = null private var allowMediaRecommendations = Utils.allowMediaRecommendations(context) + /** + * Check whether this notification is an RCN + * TODO(b/204910409) implement new API for explicitly declaring this + */ + private fun isRemoteCastNotification(sbn: StatusBarNotification): Boolean { + val pm = context.packageManager + try { + val info = pm.getApplicationInfo(sbn.packageName, PackageManager.MATCH_SYSTEM_ONLY) + if (info.privateFlags and ApplicationInfo.PRIVATE_FLAG_PRIVILEGED != 0) { + val extras = sbn.notification.extras + if (extras.containsKey(Notification.EXTRA_SUBSTITUTE_APP_NAME)) { + return true + } + } + } catch (e: PackageManager.NameNotFoundException) { } + return false + } + @Inject constructor( context: Context, @@ -442,7 +462,7 @@ class MediaDataManager( val existed = mediaEntries[key] != null backgroundExecutor.execute { mediaEntries[key]?.let { mediaData -> - if (mediaData.isLocalSession) { + if (mediaData.isLocalSession()) { mediaData.token?.let { val mediaController = mediaControllerFactory.create(it) mediaController.transportControls.stop() @@ -626,8 +646,11 @@ class MediaDataManager( } } - val isLocalSession = mediaController.playbackInfo?.playbackType == - MediaController.PlaybackInfo.PLAYBACK_TYPE_LOCAL + val playbackLocation = + if (isRemoteCastNotification(sbn)) MediaData.PLAYBACK_CAST_REMOTE + else if (mediaController.playbackInfo?.playbackType == + MediaController.PlaybackInfo.PLAYBACK_TYPE_LOCAL) MediaData.PLAYBACK_LOCAL + else MediaData.PLAYBACK_CAST_LOCAL val isPlaying = mediaController.playbackState?.let { isPlayingState(it.state) } ?: null val lastActive = systemClock.elapsedRealtime() foregroundExecutor.execute { @@ -637,7 +660,7 @@ class MediaDataManager( onMediaDataLoaded(key, oldKey, MediaData(sbn.normalizedUserId, true, bgColor, app, smallIcon, artist, song, artWorkIcon, actionIcons, actionsToShowCollapsed, sbn.packageName, token, notif.contentIntent, null, - active, resumeAction = resumeAction, isLocalSession = isLocalSession, + active, resumeAction = resumeAction, playbackLocation = playbackLocation, notificationKey = key, hasCheckedForResume = hasCheckedForResume, isPlaying = isPlaying, isClearable = sbn.isClearable(), lastActive = lastActive)) @@ -762,7 +785,7 @@ class MediaDataManager( fun onNotificationRemoved(key: String) { Assert.isMainThread() val removed = mediaEntries.remove(key) - if (useMediaResumption && removed?.resumeAction != null && removed?.isLocalSession) { + if (useMediaResumption && removed?.resumeAction != null && removed?.isLocalSession()) { Log.d(TAG, "Not removing $key because resumable") // Move to resume key (aka package name) if that key doesn't already exist. val resumeAction = getResumeMediaAction(removed.resumeAction!!) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt b/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt index 608c784f5d39..d8095f3179b3 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt @@ -193,7 +193,7 @@ class MediaResumeListener @Inject constructor( mediaBrowser = null } // If we don't have a resume action, check if we haven't already - if (data.resumeAction == null && !data.hasCheckedForResume && data.isLocalSession) { + if (data.resumeAction == null && !data.hasCheckedForResume && data.isLocalSession()) { // TODO also check for a media button receiver intended for restarting (b/154127084) Log.d(TAG, "Checking for service component for " + data.packageName) val pm = context.packageManager diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaCarouselControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaCarouselControllerTest.kt index 175ec87fd943..a6e567ea8b5a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaCarouselControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaCarouselControllerTest.kt @@ -104,37 +104,54 @@ class MediaCarouselControllerTest : SysuiTestCase() { fun testPlayerOrdering() { // Test values: key, data, last active time val playingLocal = Triple("playing local", - DATA.copy(active = true, isPlaying = true, isLocalSession = true, resumption = false), + DATA.copy(active = true, isPlaying = true, + playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false), 4500L) - val playingRemote = Triple("playing remote", - DATA.copy(active = true, isPlaying = true, isLocalSession = false, resumption = false), + val playingCast = Triple("playing cast", + DATA.copy(active = true, isPlaying = true, + playbackLocation = MediaData.PLAYBACK_CAST_LOCAL, resumption = false), 5000L) val pausedLocal = Triple("paused local", - DATA.copy(active = true, isPlaying = false, isLocalSession = true, resumption = false), + DATA.copy(active = true, isPlaying = false, + playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false), 1000L) - val pausedRemote = Triple("paused remote", - DATA.copy(active = true, isPlaying = false, isLocalSession = false, resumption = false), + val pausedCast = Triple("paused cast", + DATA.copy(active = true, isPlaying = false, + playbackLocation = MediaData.PLAYBACK_CAST_LOCAL, resumption = false), 2000L) + val playingRcn = Triple("playing RCN", + DATA.copy(active = true, isPlaying = true, + playbackLocation = MediaData.PLAYBACK_CAST_REMOTE, resumption = false), + 5000L) + + val pausedRcn = Triple("paused RCN", + DATA.copy(active = true, isPlaying = false, + playbackLocation = MediaData.PLAYBACK_CAST_REMOTE, resumption = false), + 5000L) + val resume1 = Triple("resume 1", - DATA.copy(active = false, isPlaying = false, isLocalSession = true, resumption = true), + DATA.copy(active = false, isPlaying = false, + playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = true), 500L) val resume2 = Triple("resume 2", - DATA.copy(active = false, isPlaying = false, isLocalSession = true, resumption = true), + DATA.copy(active = false, isPlaying = false, + playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = true), 1000L) // Expected ordering for media players: // Actively playing local sessions - // Actively playing remote sessions - // Paused sessions, by last active + // Actively playing cast sessions + // Paused local and cast sessions, by last active + // RCNs // Resume controls, by last active - val expected = listOf(playingLocal, playingRemote, pausedRemote, pausedLocal, resume2, - resume1) + val expected = listOf(playingLocal, playingCast, pausedCast, pausedLocal, playingRcn, + pausedRcn, resume2, resume1) expected.forEach { clock.setCurrentTimeMillis(it.third) diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java index 66b64708ad24..f870da3e68d9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java @@ -74,8 +74,8 @@ public class MediaDataCombineLatestTest extends SysuiTestCase { mManager.addListener(mListener); mMediaData = new MediaData(USER_ID, true, BG_COLOR, APP, null, ARTIST, TITLE, null, - new ArrayList<>(), new ArrayList<>(), PACKAGE, null, null, null, true, null, true, - false, KEY, false, false, false, 0L); + new ArrayList<>(), new ArrayList<>(), PACKAGE, null, null, null, true, null, + MediaData.PLAYBACK_LOCAL, false, KEY, false, false, false, 0L); mDeviceData = new MediaDeviceData(true, null, DEVICE_NAME); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt index 2b2fc513d03c..f44cc38c78c0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt @@ -1,5 +1,6 @@ package com.android.systemui.media +import android.app.Notification import android.app.Notification.MediaStyle import android.app.PendingIntent import android.app.smartspace.SmartspaceAction @@ -228,6 +229,30 @@ class MediaDataManagerTest : SysuiTestCase() { assertThat(mediaDataCaptor.value!!.active).isTrue() } + @Test + fun testOnNotificationAdded_isRcn_markedRemote() { + val bundle = Bundle().apply { + putString(Notification.EXTRA_SUBSTITUTE_APP_NAME, "Remote Cast Notification") + } + val rcn = SbnBuilder().run { + setPkg("com.android.systemui") // System package + modifyNotification(context).also { + it.setSmallIcon(android.R.drawable.ic_media_pause) + it.setStyle(MediaStyle().apply { setMediaSession(session.sessionToken) }) + it.addExtras(bundle) + } + build() + } + + mediaDataManager.onNotificationAdded(KEY, rcn) + assertThat(backgroundExecutor.runAllReady()).isEqualTo(1) + assertThat(foregroundExecutor.runAllReady()).isEqualTo(1) + verify(listener).onMediaDataLoaded(eq(KEY), eq(null), capture(mediaDataCaptor), eq(true), + eq(false)) + assertThat(mediaDataCaptor.value!!.playbackLocation).isEqualTo( + MediaData.PLAYBACK_CAST_REMOTE) + } + @Test fun testOnNotificationRemoved_callsListener() { mediaDataManager.onNotificationAdded(KEY, mediaNotification) @@ -306,7 +331,8 @@ class MediaDataManagerTest : SysuiTestCase() { verify(listener).onMediaDataLoaded(eq(KEY), eq(null), capture(mediaDataCaptor), eq(true), eq(false)) val data = mediaDataCaptor.value - val dataRemoteWithResume = data.copy(resumeAction = Runnable {}, isLocalSession = false) + val dataRemoteWithResume = data.copy(resumeAction = Runnable {}, + playbackLocation = MediaData.PLAYBACK_CAST_LOCAL) mediaDataManager.onMediaDataLoaded(KEY, null, dataRemoteWithResume) // WHEN the notification is removed diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt index 8dc9eff97ab9..421f9bee78fa 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt @@ -42,7 +42,8 @@ public class MediaPlayerDataTest : SysuiTestCase() { val mockito = MockitoJUnit.rule() companion object { - val LOCAL = true + val LOCAL = MediaData.PLAYBACK_LOCAL + val REMOTE = MediaData.PLAYBACK_CAST_LOCAL val RESUMPTION = true val PLAYING = true val UNDETERMINED = null @@ -58,7 +59,7 @@ public class MediaPlayerDataTest : SysuiTestCase() { val dataIsPlaying = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION) val playerIsRemote = mock(MediaControlPanel::class.java) - val dataIsRemote = createMediaData("app2", PLAYING, !LOCAL, !RESUMPTION) + val dataIsRemote = createMediaData("app2", PLAYING, REMOTE, !RESUMPTION) MediaPlayerData.addMediaPlayer("2", dataIsRemote, playerIsRemote, systemClock) MediaPlayerData.addMediaPlayer("1", dataIsPlaying, playerIsPlaying, systemClock) @@ -100,13 +101,13 @@ public class MediaPlayerDataTest : SysuiTestCase() { val dataIsPlaying = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION) val playerIsPlayingAndRemote = mock(MediaControlPanel::class.java) - val dataIsPlayingAndRemote = createMediaData("app2", PLAYING, !LOCAL, !RESUMPTION) + val dataIsPlayingAndRemote = createMediaData("app2", PLAYING, REMOTE, !RESUMPTION) val playerIsStoppedAndLocal = mock(MediaControlPanel::class.java) val dataIsStoppedAndLocal = createMediaData("app3", !PLAYING, LOCAL, !RESUMPTION) val playerIsStoppedAndRemote = mock(MediaControlPanel::class.java) - val dataIsStoppedAndRemote = createMediaData("app4", !PLAYING, !LOCAL, !RESUMPTION) + val dataIsStoppedAndRemote = createMediaData("app4", !PLAYING, REMOTE, !RESUMPTION) val playerCanResume = mock(MediaControlPanel::class.java) val dataCanResume = createMediaData("app5", !PLAYING, LOCAL, RESUMPTION) @@ -127,8 +128,8 @@ public class MediaPlayerDataTest : SysuiTestCase() { val players = MediaPlayerData.players() assertThat(players).hasSize(6) assertThat(players).containsExactly(playerIsPlaying, playerIsPlayingAndRemote, - playerIsStoppedAndRemote, playerIsStoppedAndLocal, playerCanResume, - playerUndetermined).inOrder() + playerIsStoppedAndRemote, playerIsStoppedAndLocal, playerUndetermined, + playerCanResume).inOrder() } @Test @@ -160,9 +161,10 @@ public class MediaPlayerDataTest : SysuiTestCase() { private fun createMediaData( app: String, isPlaying: Boolean?, - isLocalSession: Boolean, + location: Int, resumption: Boolean ) = - MediaData(0, false, 0, app, null, null, null, null, emptyList(), emptyList(), "", - null, null, null, true, null, isLocalSession, resumption, null, false, isPlaying) + MediaData(0, false, 0, app, null, null, null, null, emptyList(), emptyList(), + "package:" + app, null, null, null, true, null, location, resumption, "key:" + app, + false, isPlaying) } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt index 359746bca0ff..54da1a4b5f65 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt @@ -211,10 +211,20 @@ class MediaResumeListenerTest : SysuiTestCase() { } @Test - fun testOnLoad_remotePlayback_doesNotCheck() { - // When media data is loaded that has not been checked yet, and is not local - val dataRemote = data.copy(isLocalSession = false) - resumeListener.onMediaDataLoaded(KEY, null, dataRemote) + fun testOnLoad_localCast_doesNotCheck() { + // When media data is loaded that has not been checked yet, and is a local cast + val dataCast = data.copy(playbackLocation = MediaData.PLAYBACK_CAST_LOCAL) + resumeListener.onMediaDataLoaded(KEY, null, dataCast) + + // Then we do not take action + verify(mediaDataManager, never()).setResumeAction(any(), any()) + } + + @Test + fun testOnload_remoteCast_doesNotCheck() { + // When media data is loaded that has not been checked yet, and is a remote cast + val dataRcn = data.copy(playbackLocation = MediaData.PLAYBACK_CAST_REMOTE) + resumeListener.onMediaDataLoaded(KEY, null, dataRcn) // Then we do not take action verify(mediaDataManager, never()).setResumeAction(any(), any()) -- cgit v1.2.3-59-g8ed1b