From b0f2e4b5d2bfe5ebfac66f31c010055db78c3e01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Go=CC=88llner?= Date: Mon, 15 Aug 2022 11:31:51 +0200 Subject: [PATCH] Fix media staying too long during shade "immediate" expansion/collapsing The problem was that the media player was doing a guided transitioning when the shade was going directly to fully expanded or fully collapsed. The transition is supposed to only happen when going from QQS to QS and vice versa. This doesn't apply to split shade, as there it QQS doesn't exist, but media should still transition. Change-Id: I8b9646d752440e2ac3274c3cbb98f9002b47e6cd Test: MediaHierarchyManagerTest.kt Test: Manually Fixes: 233178720 --- .../systemui/media/MediaHierarchyManager.kt | 31 ++++++- .../systemui/shade/NotifPanelEvents.kt | 18 +++- .../NotificationPanelViewController.java | 23 ++++-- .../media/MediaHierarchyManagerTest.kt | 82 ++++++++++++------- .../shade/testing/FakeNotifPanelEvents.kt | 37 +++++++++ 5 files changed, 151 insertions(+), 40 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/shade/testing/FakeNotifPanelEvents.kt diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt index 458ed4059b3be..926002060da7a 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaHierarchyManager.kt @@ -36,6 +36,7 @@ import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dreams.DreamOverlayStateController import com.android.systemui.keyguard.WakefulnessLifecycle import com.android.systemui.plugins.statusbar.StatusBarStateController +import com.android.systemui.shade.NotifPanelEvents import com.android.systemui.statusbar.CrossFadeHelper import com.android.systemui.statusbar.NotificationLockscreenUserManager import com.android.systemui.statusbar.StatusBarState @@ -85,12 +86,21 @@ class MediaHierarchyManager @Inject constructor( private val bypassController: KeyguardBypassController, private val mediaCarouselController: MediaCarouselController, private val notifLockscreenUserManager: NotificationLockscreenUserManager, + private val keyguardViewController: KeyguardViewController, + private val dreamOverlayStateController: DreamOverlayStateController, configurationController: ConfigurationController, wakefulnessLifecycle: WakefulnessLifecycle, - private val keyguardViewController: KeyguardViewController, - private val dreamOverlayStateController: DreamOverlayStateController + panelEventsEvents: NotifPanelEvents, ) { + /** + * Whether we "skip" QQS during panel expansion. + * + * This means that when expanding the panel we go directly to QS. Also when we are on QS and + * start closing the panel, it fully collapses instead of going to QQS. + */ + private var skipQqsOnExpansion: Boolean = false + /** * The root overlay of the hierarchy. This is where the media notification is attached to * whenever the view is transitioning from one host to another. It also make sure that the @@ -504,6 +514,13 @@ class MediaHierarchyManager @Inject constructor( mediaCarouselController.updateUserVisibility = { mediaCarouselController.mediaCarouselScrollHandler.visibleToUser = isVisibleToUser() } + + panelEventsEvents.registerListener(object : NotifPanelEvents.Listener { + override fun onExpandImmediateChanged(isExpandImmediateEnabled: Boolean) { + skipQqsOnExpansion = isExpandImmediateEnabled + updateDesiredLocation() + } + }) } private fun updateConfiguration() { @@ -701,6 +718,9 @@ class MediaHierarchyManager @Inject constructor( if (isCurrentlyInGuidedTransformation()) { return false } + if (skipQqsOnExpansion) { + return false + } // This is an invalid transition, and can happen when using the camera gesture from the // lock screen. Disallow. if (previousLocation == LOCATION_LOCKSCREEN && @@ -852,6 +872,9 @@ class MediaHierarchyManager @Inject constructor( * otherwise */ private fun getTransformationProgress(): Float { + if (skipQqsOnExpansion) { + return -1.0f + } val progress = getQSTransformationProgress() if (statusbarState != StatusBarState.KEYGUARD && progress >= 0) { return progress @@ -1042,6 +1065,10 @@ class MediaHierarchyManager @Inject constructor( // reattach it without an animation return LOCATION_LOCKSCREEN } + if (skipQqsOnExpansion) { + // When doing an immediate expand or collapse, we want to keep it in QS. + return LOCATION_QS + } return location } diff --git a/packages/SystemUI/src/com/android/systemui/shade/NotifPanelEvents.kt b/packages/SystemUI/src/com/android/systemui/shade/NotifPanelEvents.kt index ce9d89f89ae14..4558061de1a24 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/NotifPanelEvents.kt +++ b/packages/SystemUI/src/com/android/systemui/shade/NotifPanelEvents.kt @@ -29,11 +29,25 @@ interface NotifPanelEvents { interface Listener { /** Invoked when the notification panel starts or stops collapsing. */ - fun onPanelCollapsingChanged(isCollapsing: Boolean) + @JvmDefault + fun onPanelCollapsingChanged(isCollapsing: Boolean) {} /** * Invoked when the notification panel starts or stops launching an [android.app.Activity]. */ - fun onLaunchingActivityChanged(isLaunchingActivity: Boolean) + @JvmDefault + fun onLaunchingActivityChanged(isLaunchingActivity: Boolean) {} + + /** + * Invoked when the "expand immediate" attribute changes. + * + * An example of expanding immediately is when swiping down from the top with two fingers. + * Instead of going to QQS, we immediately expand to full QS. + * + * Another example is when full QS is showing, and we swipe up from the bottom. Instead of + * going to QQS, the panel fully collapses. + */ + @JvmDefault + fun onExpandImmediateChanged(isExpandImmediateEnabled: Boolean) {} } } diff --git a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java index a353e8fee9458..20936c40c151d 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java @@ -1702,12 +1702,17 @@ public final class NotificationPanelViewController extends PanelViewController { } if (mQsExpanded) { - mQsExpandImmediate = true; + setQsExpandImmediate(true); setShowShelfOnly(true); } super.collapse(delayed, speedUpFactor); } + private void setQsExpandImmediate(boolean expandImmediate) { + mQsExpandImmediate = expandImmediate; + mPanelEventsEmitter.notifyExpandImmediateChange(expandImmediate); + } + private void setShowShelfOnly(boolean shelfOnly) { mNotificationStackScrollLayoutController.setShouldShowShelfOnly( shelfOnly && !mSplitShadeEnabled); @@ -1760,7 +1765,7 @@ public final class NotificationPanelViewController extends PanelViewController { public void expandWithQs() { if (isQsExpansionEnabled()) { - mQsExpandImmediate = true; + setQsExpandImmediate(true); setShowShelfOnly(true); } if (mSplitShadeEnabled && isOnKeyguard()) { @@ -2089,7 +2094,7 @@ public final class NotificationPanelViewController extends PanelViewController { if (mTwoFingerQsExpandPossible && isOpenQsEvent(event) && event.getY(event.getActionIndex()) < mStatusBarMinHeight) { mMetricsLogger.count(COUNTER_PANEL_OPEN_QS, 1); - mQsExpandImmediate = true; + setQsExpandImmediate(true); setShowShelfOnly(true); requestPanelHeightUpdate(); @@ -3291,7 +3296,7 @@ public final class NotificationPanelViewController extends PanelViewController { } else { setListening(true); } - mQsExpandImmediate = false; + setQsExpandImmediate(false); setShowShelfOnly(false); mTwoFingerQsExpandPossible = false; updateTrackingHeadsUp(null); @@ -3349,7 +3354,7 @@ public final class NotificationPanelViewController extends PanelViewController { super.onTrackingStarted(); mScrimController.onTrackingStarted(); if (mQsFullyExpanded) { - mQsExpandImmediate = true; + setQsExpandImmediate(true); setShowShelfOnly(true); } mNotificationStackScrollLayoutController.onPanelTrackingStarted(); @@ -4914,7 +4919,7 @@ public final class NotificationPanelViewController extends PanelViewController { // to locked will trigger this event and we're not actually in the process of opening // the shade, lockscreen is just always expanded if (mSplitShadeEnabled && !isOnKeyguard()) { - mQsExpandImmediate = true; + setQsExpandImmediate(true); } mCentralSurfaces.makeExpandedVisible(false); } @@ -4981,5 +4986,11 @@ public final class NotificationPanelViewController extends PanelViewController { cb.onPanelCollapsingChanged(isCollapsing); } } + + private void notifyExpandImmediateChange(boolean expandImmediateEnabled) { + for (NotifPanelEvents.Listener cb : mListeners) { + cb.onExpandImmediateChanged(expandImmediateEnabled); + } + } } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaHierarchyManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaHierarchyManagerTest.kt index d65b6b31a26d0..369913d1ea73b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaHierarchyManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaHierarchyManagerTest.kt @@ -16,7 +16,6 @@ package com.android.systemui.media -import org.mockito.Mockito.`when` as whenever import android.graphics.Rect import android.testing.AndroidTestingRunner import android.testing.TestableLooper @@ -30,6 +29,7 @@ import com.android.systemui.controls.controller.ControlsControllerImplTest.Compa import com.android.systemui.dreams.DreamOverlayStateController import com.android.systemui.keyguard.WakefulnessLifecycle import com.android.systemui.plugins.statusbar.StatusBarStateController +import com.android.systemui.shade.testing.FakeNotifPanelEvents import com.android.systemui.statusbar.NotificationLockscreenUserManager import com.android.systemui.statusbar.StatusBarState import com.android.systemui.statusbar.SysuiStatusBarStateController @@ -50,10 +50,11 @@ import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.ArgumentMatchers.anyLong import org.mockito.Captor import org.mockito.Mock -import org.mockito.Mockito.`when` import org.mockito.Mockito.clearInvocations import org.mockito.Mockito.times import org.mockito.Mockito.verify +import org.mockito.Mockito.`when` +import org.mockito.Mockito.`when` as whenever import org.mockito.junit.MockitoJUnit @SmallTest @@ -61,32 +62,19 @@ import org.mockito.junit.MockitoJUnit @TestableLooper.RunWithLooper class MediaHierarchyManagerTest : SysuiTestCase() { - @Mock - private lateinit var lockHost: MediaHost - @Mock - private lateinit var qsHost: MediaHost - @Mock - private lateinit var qqsHost: MediaHost - @Mock - private lateinit var bypassController: KeyguardBypassController - @Mock - private lateinit var keyguardStateController: KeyguardStateController - @Mock - private lateinit var statusBarStateController: SysuiStatusBarStateController - @Mock - private lateinit var notificationLockscreenUserManager: NotificationLockscreenUserManager - @Mock - private lateinit var mediaCarouselController: MediaCarouselController - @Mock - private lateinit var mediaCarouselScrollHandler: MediaCarouselScrollHandler - @Mock - private lateinit var wakefulnessLifecycle: WakefulnessLifecycle - @Mock - private lateinit var keyguardViewController: KeyguardViewController - @Mock - private lateinit var uniqueObjectHostView: UniqueObjectHostView - @Mock - private lateinit var dreamOverlayStateController: DreamOverlayStateController + @Mock private lateinit var lockHost: MediaHost + @Mock private lateinit var qsHost: MediaHost + @Mock private lateinit var qqsHost: MediaHost + @Mock private lateinit var bypassController: KeyguardBypassController + @Mock private lateinit var keyguardStateController: KeyguardStateController + @Mock private lateinit var statusBarStateController: SysuiStatusBarStateController + @Mock private lateinit var notificationLockscreenUserManager: NotificationLockscreenUserManager + @Mock private lateinit var mediaCarouselController: MediaCarouselController + @Mock private lateinit var mediaCarouselScrollHandler: MediaCarouselScrollHandler + @Mock private lateinit var wakefulnessLifecycle: WakefulnessLifecycle + @Mock private lateinit var keyguardViewController: KeyguardViewController + @Mock private lateinit var uniqueObjectHostView: UniqueObjectHostView + @Mock private lateinit var dreamOverlayStateController: DreamOverlayStateController @Captor private lateinit var wakefullnessObserver: ArgumentCaptor<(WakefulnessLifecycle.Observer)> @Captor @@ -97,6 +85,7 @@ class MediaHierarchyManagerTest : SysuiTestCase() { private lateinit var mediaHiearchyManager: MediaHierarchyManager private lateinit var mediaFrame: ViewGroup private val configurationController = FakeConfigurationController() + private val notifPanelEvents = FakeNotifPanelEvents() @Before fun setup() { @@ -111,10 +100,12 @@ class MediaHierarchyManagerTest : SysuiTestCase() { bypassController, mediaCarouselController, notificationLockscreenUserManager, + keyguardViewController, + dreamOverlayStateController, configurationController, wakefulnessLifecycle, - keyguardViewController, - dreamOverlayStateController) + notifPanelEvents, + ) verify(wakefulnessLifecycle).addObserver(wakefullnessObserver.capture()) verify(statusBarStateController).addCallback(statusBarCallback.capture()) setupHost(lockHost, MediaHierarchyManager.LOCATION_LOCKSCREEN, LOCKSCREEN_TOP) @@ -211,6 +202,25 @@ class MediaHierarchyManagerTest : SysuiTestCase() { assertThat(transformType).isEqualTo(MediaHierarchyManager.TRANSFORMATION_TYPE_FADE) } + @Test + fun calculateTransformationType_notOnLockscreen_returnsTransition() { + expandQS() + + val transformType = mediaHiearchyManager.calculateTransformationType() + + assertThat(transformType).isEqualTo(MediaHierarchyManager.TRANSFORMATION_TYPE_TRANSITION) + } + + @Test + fun calculateTransformationType_onLockscreen_returnsTransition() { + goToLockscreen() + expandQS() + + val transformType = mediaHiearchyManager.calculateTransformationType() + + assertThat(transformType).isEqualTo(MediaHierarchyManager.TRANSFORMATION_TYPE_FADE) + } + @Test fun calculateTransformationType_onLockShade_inSplitShade_goingToFullShade_returnsTransition() { enableSplitShade() @@ -294,6 +304,18 @@ class MediaHierarchyManagerTest : SysuiTestCase() { assertThat(mediaHiearchyManager.isCurrentlyInGuidedTransformation()).isTrue() } + @Test + fun isCurrentlyInGuidedTransformation_hostsVisible_expandImmediateEnabled_returnsFalse() { + notifPanelEvents.changeExpandImmediate(expandImmediate = true) + goToLockscreen() + enterGuidedTransformation() + whenever(lockHost.visible).thenReturn(true) + whenever(qsHost.visible).thenReturn(true) + whenever(qqsHost.visible).thenReturn(true) + + assertThat(mediaHiearchyManager.isCurrentlyInGuidedTransformation()).isFalse() + } + @Test fun isCurrentlyInGuidedTransformation_hostNotVisible_returnsTrue() { goToLockscreen() diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/testing/FakeNotifPanelEvents.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/testing/FakeNotifPanelEvents.kt new file mode 100644 index 0000000000000..d052138772325 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/testing/FakeNotifPanelEvents.kt @@ -0,0 +1,37 @@ +/* + * 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.shade.testing + +import com.android.systemui.shade.NotifPanelEvents + +/** Fake implementation of [NotifPanelEvents] for testing. */ +class FakeNotifPanelEvents : NotifPanelEvents { + + private val listeners = mutableListOf() + + override fun registerListener(listener: NotifPanelEvents.Listener) { + listeners.add(listener) + } + + override fun unregisterListener(listener: NotifPanelEvents.Listener) { + listeners.remove(listener) + } + + fun changeExpandImmediate(expandImmediate: Boolean) { + listeners.forEach { it.onExpandImmediateChanged(expandImmediate) } + } +}