From 7ba605bce50ba8831179d2380f89fcc5b5c0d957 Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Fri, 19 Jun 2020 18:34:23 -0700 Subject: [PATCH 1/2] Fixed a bug where the player was invisible We were blocking location changes because of visibility of hosts, but never listened to changes to it Bug: 159494391 Test: kill sysui with player Change-Id: I8a195b3e5a91210ed1c03a24d09f7bda5923b210 --- .../android/systemui/media/KeyguardMediaController.kt | 2 +- .../android/systemui/media/MediaHierarchyManager.kt | 11 +++++++++-- .../src/com/android/systemui/media/MediaHost.kt | 11 +++++++++-- .../SystemUI/src/com/android/systemui/qs/QSPanel.java | 2 +- .../systemui/media/KeyguardMediaControllerTest.kt | 3 ++- 5 files changed, 22 insertions(+), 7 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/KeyguardMediaController.kt b/packages/SystemUI/src/com/android/systemui/media/KeyguardMediaController.kt index f8c2b88d39de0..8ef20f89085c7 100644 --- a/packages/SystemUI/src/com/android/systemui/media/KeyguardMediaController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/KeyguardMediaController.kt @@ -56,7 +56,7 @@ class KeyguardMediaController @Inject constructor( fun attach(mediaView: MediaHeaderView) { view = mediaView // First let's set the desired state that we want for this host - mediaHost.visibleChangedListener = { updateVisibility() } + mediaHost.addVisibilityChangeListener { updateVisibility() } mediaHost.expansion = 0.0f mediaHost.showsOnlyActiveMedia = true mediaHost.falsingProtectionNeeded = true diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt index c378c8b7d0980..3266074eba7de 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt @@ -227,6 +227,10 @@ class MediaHierarchyManager @Inject constructor( fun register(mediaObject: MediaHost): UniqueObjectHostView { val viewHost = createUniqueObjectHost() mediaObject.hostView = viewHost + mediaObject.addVisibilityChangeListener { + // Never animate because of a visibility change, only state changes should do that + updateDesiredLocation(forceNoAnimation = true) + } mediaHosts[mediaObject.location] = mediaObject if (mediaObject.location == desiredLocation) { // In case we are overriding a view that is already visible, make sure we attach it @@ -260,8 +264,10 @@ class MediaHierarchyManager @Inject constructor( /** * Updates the location that the view should be in. If it changes, an animation may be triggered * going from the old desired location to the new one. + * + * @param forceNoAnimation optional parameter telling the system not to animate */ - private fun updateDesiredLocation() { + private fun updateDesiredLocation(forceNoAnimation: Boolean = false) { val desiredLocation = calculateLocation() if (desiredLocation != this.desiredLocation) { if (this.desiredLocation >= 0) { @@ -270,7 +276,8 @@ class MediaHierarchyManager @Inject constructor( val isNewView = this.desiredLocation == -1 this.desiredLocation = desiredLocation // Let's perform a transition - val animate = shouldAnimateTransition(desiredLocation, previousLocation) + val animate = !forceNoAnimation && + shouldAnimateTransition(desiredLocation, previousLocation) val (animDuration, delay) = getAnimationParams(previousLocation, desiredLocation) val host = getHost(desiredLocation) mediaCarouselController.onDesiredLocationChanged(desiredLocation, host, animate, diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt b/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt index 19eb04b10ce34..1ae9d3ff4ca53 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaHost.kt @@ -2,6 +2,7 @@ package com.android.systemui.media import android.graphics.PointF import android.graphics.Rect +import android.util.ArraySet import android.view.View import android.view.View.OnAttachStateChangeListener import com.android.systemui.util.animation.MeasurementInput @@ -20,7 +21,7 @@ class MediaHost @Inject constructor( lateinit var hostView: UniqueObjectHostView var location: Int = -1 private set - var visibleChangedListener: ((Boolean) -> Unit)? = null + private var visibleChangedListeners: ArraySet<(Boolean) -> Unit> = ArraySet() private val tmpLocationOnScreen: IntArray = intArrayOf(0, 0) @@ -58,6 +59,10 @@ class MediaHost @Inject constructor( } } + fun addVisibilityChangeListener(listener: (Boolean) -> Unit) { + visibleChangedListeners.add(listener) + } + /** * Initialize this MediaObject and create a host view. * All state should already be set on this host before calling this method in order to avoid @@ -116,7 +121,9 @@ class MediaHost @Inject constructor( val newVisibility = if (visible) View.VISIBLE else View.GONE if (newVisibility != hostView.visibility) { hostView.visibility = newVisibility - visibleChangedListener?.invoke(visible) + visibleChangedListeners.forEach { + it.invoke(visible) + } } } diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java b/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java index d21fbec2b99b7..5f191062c0d9c 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java @@ -160,7 +160,7 @@ public class QSPanel extends LinearLayout implements Tunable, Callback, Brightne mMediaTotalBottomMargin = getResources().getDimensionPixelSize( R.dimen.quick_settings_bottom_margin_media); mMediaHost = mediaHost; - mMediaHost.setVisibleChangedListener((visible) -> { + mMediaHost.addVisibilityChangeListener((visible) -> { onMediaVisibilityChanged(visible); return null; }); diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/KeyguardMediaControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/KeyguardMediaControllerTest.kt index 9aee11e4924ff..008dc12bba03a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/KeyguardMediaControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/KeyguardMediaControllerTest.kt @@ -27,6 +27,7 @@ import com.android.systemui.statusbar.StatusBarState import com.android.systemui.statusbar.SysuiStatusBarStateController import com.android.systemui.statusbar.notification.stack.MediaHeaderView import com.android.systemui.statusbar.phone.KeyguardBypassController +import com.android.systemui.util.mockito.capture import org.junit.Before import org.junit.Rule import org.junit.Test @@ -94,7 +95,7 @@ class KeyguardMediaControllerTest : SysuiTestCase() { private fun triggerVisibilityListener() { keyguardMediaController.attach(mediaHeaderView) - verify(mediaHost).visibleChangedListener = visibilityListener.capture() + verify(mediaHost).addVisibilityChangeListener(capture(visibilityListener)) visibilityListener.value.invoke(true) } } \ No newline at end of file From 663e1c762ea8f71c9cb03e2f270ba5a7f03a7636 Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Fri, 19 Jun 2020 17:01:44 -0700 Subject: [PATCH 2/2] Enable drag down with only media Without notifications, users couldn't drag down Fixes: 159495916 Test: add media, no notifications, drag down Change-Id: I06d9123410dc700fa3dad515a3f73b222df088cf --- .../notification/stack/NotificationStackScrollLayout.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java index 1a12d199c55ae..8ebadfbd3b254 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java @@ -201,6 +201,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd private final KeyguardBypassController mKeyguardBypassController; private final DynamicPrivacyController mDynamicPrivacyController; private final SysuiStatusBarStateController mStatusbarStateController; + private final KeyguardMediaController mKeyguardMediaController; private ExpandHelper mExpandHelper; private final NotificationSwipeHelper mSwipeHelper; @@ -672,6 +673,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd initializeForegroundServiceSection(fgsFeatureController); mUiEventLogger = uiEventLogger; mColorExtractor.addOnColorsChangedListener(mOnColorsChangedListener); + mKeyguardMediaController = keyguardMediaController; keyguardMediaController.setVisibilityChangedListener((visible) -> { if (visible) { generateAddAnimation(keyguardMediaController.getView(), false /*fromMoreCard */); @@ -6638,7 +6640,9 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd /* Only ever called as a consequence of a lockscreen expansion gesture. */ @Override public boolean onDraggedDown(View startingChild, int dragLengthY) { - if (mStatusBarState == StatusBarState.KEYGUARD && hasActiveNotifications()) { + boolean canDragDown = hasActiveNotifications() + || mKeyguardMediaController.getView().getVisibility() == VISIBLE; + if (mStatusBarState == StatusBarState.KEYGUARD && canDragDown) { mLockscreenGestureLogger.write( MetricsEvent.ACTION_LS_SHADE, (int) (dragLengthY / mDisplayMetrics.density),