From 734c374976d5553744367fbc82db8c543ee00729 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Mon, 28 Sep 2020 13:14:28 -0400 Subject: [PATCH 1/3] Media player attempts to dismiss notifications when players removed. NOTE: this is only a partial fix. See bug for discussion. (Cherry-picked from 725d67d88940fa03cfaac9f3df8b472a1a68f77d) Bug: 169271494 Test: manual Change-Id: I64db4cc618a812063c18a40ea4465aa4c520a3be Merged-In: I64db4cc618a812063c18a40ea4465aa4c520a3be --- .../statusbar/NotificationMediaManager.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java index 8cd82cc775304..38e1de3cada06 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationMediaManager.java @@ -40,6 +40,7 @@ import android.os.Trace; import android.os.UserHandle; import android.provider.DeviceConfig; import android.provider.DeviceConfig.Properties; +import android.service.notification.NotificationListenerService; import android.util.ArraySet; import android.util.Log; import android.view.View; @@ -52,6 +53,7 @@ import com.android.systemui.Dumpable; import com.android.systemui.Interpolators; import com.android.systemui.colorextraction.SysuiColorExtractor; import com.android.systemui.dagger.qualifiers.Main; +import com.android.systemui.media.MediaData; import com.android.systemui.media.MediaDataManager; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.dagger.StatusBarModule; @@ -250,6 +252,24 @@ public class NotificationMediaManager implements Dumpable { } }); + mMediaDataManager.addListener(new MediaDataManager.Listener() { + @Override + public void onMediaDataLoaded(@NonNull String key, + @Nullable String oldKey, @NonNull MediaData data) { + } + + @Override + public void onMediaDataRemoved(@NonNull String key) { + NotificationEntry entry = mEntryManager.getPendingOrActiveNotif(key); + if (entry != null) { + // TODO(b/160713608): "removing" this notification won't happen and + // won't send the 'deleteIntent' if the notification is ongoing. + mEntryManager.performRemoveNotification(entry.getSbn(), + NotificationListenerService.REASON_CANCEL); + } + } + }); + mShowCompactMediaSeekbar = "true".equals( DeviceConfig.getProperty(DeviceConfig.NAMESPACE_SYSTEMUI, SystemUiDeviceConfigFlags.COMPACT_MEDIA_SEEKBAR_ENABLED)); From a49a320e4935f4ffbea0dc31448b90adcac45730 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Mon, 12 Oct 2020 16:34:55 -0400 Subject: [PATCH 2/3] Disable player's "Dismiss" button when notification is not dismissible. Bug: 169271494 Test: manual Change-Id: I3c2a73e4885642965ec553387b5e91f0cb16ff2f Merged-In: I3c2a73e4885642965ec553387b5e91f0cb16ff2f --- packages/SystemUI/res/values/strings.xml | 2 ++ .../systemui/media/MediaControlPanel.java | 8 +++++++ .../systemui/media/PlayerViewHolder.kt | 4 +++- .../systemui/media/MediaControlPanelTest.kt | 23 +++++++++++++++++-- 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/res/values/strings.xml b/packages/SystemUI/res/values/strings.xml index 77ce39fe99534..95de4860ddfa7 100644 --- a/packages/SystemUI/res/values/strings.xml +++ b/packages/SystemUI/res/values/strings.xml @@ -2820,6 +2820,8 @@ Media Hide the current session. + + Current session cannot be hidden. Dismiss diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java index 810cecca517fb..d853e3d4a57c4 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java +++ b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java @@ -359,7 +359,15 @@ public class MediaControlPanel { final MediaController controller = getController(); mBackgroundExecutor.execute(() -> mSeekBarViewModel.updateController(controller)); + // Guts label + boolean isDismissible = data.isClearable(); + mViewHolder.getSettingsText().setText(isDismissible + ? R.string.controls_media_close_session + : R.string.controls_media_active_session); + // Dismiss + mViewHolder.getDismissLabel().setAlpha(isDismissible ? 1 : DISABLED_ALPHA); + mViewHolder.getDismiss().setEnabled(isDismissible); mViewHolder.getDismiss().setOnClickListener(v -> { if (mKey != null) { closeGuts(); diff --git a/packages/SystemUI/src/com/android/systemui/media/PlayerViewHolder.kt b/packages/SystemUI/src/com/android/systemui/media/PlayerViewHolder.kt index 666a6038a8b68..16327bd9064a7 100644 --- a/packages/SystemUI/src/com/android/systemui/media/PlayerViewHolder.kt +++ b/packages/SystemUI/src/com/android/systemui/media/PlayerViewHolder.kt @@ -60,8 +60,10 @@ class PlayerViewHolder private constructor(itemView: View) { val action4 = itemView.requireViewById(R.id.action4) // Settings screen + val settingsText = itemView.requireViewById(R.id.remove_text) val cancel = itemView.requireViewById(R.id.cancel) - val dismiss = itemView.requireViewById(R.id.dismiss) + val dismiss = itemView.requireViewById(R.id.dismiss) + val dismissLabel = dismiss.getChildAt(0) val settings = itemView.requireViewById(R.id.settings) init { diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt index 81139f192070f..ad703614fdb48 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt @@ -109,9 +109,11 @@ public class MediaControlPanelTest : SysuiTestCase() { private lateinit var action2: ImageButton private lateinit var action3: ImageButton private lateinit var action4: ImageButton + private lateinit var settingsText: TextView private lateinit var settings: View private lateinit var cancel: View - private lateinit var dismiss: View + private lateinit var dismiss: FrameLayout + private lateinit var dismissLabel: View private lateinit var session: MediaSession private val device = MediaDeviceData(true, null, DEVICE_NAME) @@ -168,12 +170,16 @@ public class MediaControlPanelTest : SysuiTestCase() { whenever(holder.action3).thenReturn(action3) action4 = ImageButton(context) whenever(holder.action4).thenReturn(action4) + settingsText = TextView(context) + whenever(holder.settingsText).thenReturn(settingsText) settings = View(context) whenever(holder.settings).thenReturn(settings) cancel = View(context) whenever(holder.cancel).thenReturn(cancel) - dismiss = View(context) + dismiss = FrameLayout(context) whenever(holder.dismiss).thenReturn(dismiss) + dismissLabel = View(context) + whenever(holder.dismissLabel).thenReturn(dismissLabel) // Create media session val metadataBuilder = MediaMetadata.Builder().apply { @@ -327,6 +333,7 @@ public class MediaControlPanelTest : SysuiTestCase() { notificationKey = KEY) player.bind(state, mediaKey) + assertThat(dismiss.isEnabled).isEqualTo(true) dismiss.callOnClick() val captor = ArgumentCaptor.forClass(ActivityStarter.OnDismissAction::class.java) verify(keyguardDismissUtil).executeWhenUnlocked(captor.capture(), anyBoolean()) @@ -334,4 +341,16 @@ public class MediaControlPanelTest : SysuiTestCase() { captor.value.onDismiss() verify(mediaDataManager).dismissMediaData(eq(mediaKey), anyLong()) } + + @Test + fun dismissButtonDisabled() { + val mediaKey = "key for dismissal" + player.attach(holder) + val state = MediaData(USER_ID, true, BG_COLOR, APP, null, ARTIST, TITLE, null, emptyList(), + emptyList(), PACKAGE, session.getSessionToken(), null, null, true, null, + isClearable = false, notificationKey = KEY) + player.bind(state, mediaKey) + + assertThat(dismiss.isEnabled).isEqualTo(false) + } } From fb21535b26a7ae93e860050d4f3cfaf9f29c3699 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Mon, 12 Oct 2020 14:11:22 -0400 Subject: [PATCH 3/3] Notifications starting paused shall still be subject to timeout. Bug: 169271494 Test: manual Change-Id: Iebb4f113d73179d7314ae1c6db3856ee0fd10b07 Merged-In: Iebb4f113d73179d7314ae1c6db3856ee0fd10b07 --- .../com/android/systemui/media/MediaTimeoutListener.kt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt b/packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt index dcb7767a680a3..63a361de4d87d 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt @@ -46,7 +46,7 @@ class MediaTimeoutListener @Inject constructor( /** * Callback representing that a media object is now expired: * @param token Media session unique identifier - * @param pauseTimeuot True when expired for {@code PAUSED_MEDIA_TIMEOUT} + * @param pauseTimeout True when expired for {@code PAUSED_MEDIA_TIMEOUT} */ lateinit var timeoutCallback: (String, Boolean) -> Unit @@ -57,11 +57,10 @@ class MediaTimeoutListener @Inject constructor( // Having an old key means that we're migrating from/to resumption. We should update // the old listener to make sure that events will be dispatched to the new location. val migrating = oldKey != null && key != oldKey - var wasPlaying = false if (migrating) { val reusedListener = mediaListeners.remove(oldKey) if (reusedListener != null) { - wasPlaying = reusedListener.playing ?: false + val wasPlaying = reusedListener.playing ?: false if (DEBUG) Log.d(TAG, "migrating key $oldKey to $key, for resumption") reusedListener.mediaData = data reusedListener.key = key @@ -159,9 +158,8 @@ class MediaTimeoutListener @Inject constructor( Log.v(TAG, "Execute timeout for $key") } timedOut = true - if (dispatchEvents) { - timeoutCallback(key, timedOut) - } + // this event is async, so it's safe even when `dispatchEvents` is false + timeoutCallback(key, timedOut) }, PAUSED_MEDIA_TIMEOUT) } else { expireMediaTimeout(key, "playback started - $state, $key")