Merge "Fixed an issue where the keyguard media visibility was wrong" into sc-dev

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

@@ -16,16 +16,22 @@
 
 package com.android.systemui.media
 
+import android.content.Context
+import android.content.res.Configuration
 import android.view.View
 import android.view.ViewGroup
+import androidx.annotation.VisibleForTesting
 import com.android.systemui.dagger.SysUISingleton
 import com.android.systemui.media.dagger.MediaModule.KEYGUARD
 import com.android.systemui.plugins.statusbar.StatusBarStateController
+import com.android.systemui.statusbar.FeatureFlags
 import com.android.systemui.statusbar.NotificationLockscreenUserManager
 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.statusbar.policy.ConfigurationController
+import com.android.systemui.util.Utils
 import javax.inject.Inject
 import javax.inject.Named
 
@@ -38,7 +44,10 @@ class KeyguardMediaController @Inject constructor(
     @param:Named(KEYGUARD) private val mediaHost: MediaHost,
     private val bypassController: KeyguardBypassController,
     private val statusBarStateController: SysuiStatusBarStateController,
-    private val notifLockscreenUserManager: NotificationLockscreenUserManager
+    private val notifLockscreenUserManager: NotificationLockscreenUserManager,
+    private val featureFlags: FeatureFlags,
+    private val context: Context,
+    configurationController: ConfigurationController
 ) {
 
     init {
@@ -46,12 +55,10 @@ class KeyguardMediaController @Inject constructor(
             override fun onStateChanged(newState: Int) {
                 refreshMediaPosition()
             }
-
+        })
-            override fun onDozingChanged(isDozing: Boolean) {
+        configurationController.addCallback(object : ConfigurationController.ConfigurationListener {
-                if (!isDozing) {
+            override fun onConfigChanged(newConfig: Configuration?) {
-                    mediaHost.visible = true
+                updateResources()
-                    refreshMediaPosition()
-                }
             }
         })
 
@@ -62,8 +69,24 @@ class KeyguardMediaController @Inject constructor(
 
         // Let's now initialize this view, which also creates the host view for us.
         mediaHost.init(MediaHierarchyManager.LOCATION_LOCKSCREEN)
+        updateResources()
     }
 
+    private fun updateResources() {
+        useSplitShade = Utils.shouldUseSplitNotificationShade(featureFlags, context.resources)
+    }
+
+    @VisibleForTesting
+    var useSplitShade = false
+        set(value) {
+            if (field == value) {
+                return
+            }
+            field = value
+            reattachHostView()
+            refreshMediaPosition()
+        }
+
     /**
      * Is the media player visible?
      */
@@ -78,34 +101,63 @@ class KeyguardMediaController @Inject constructor(
     var singlePaneContainer: MediaHeaderView? = null
         private set
     private var splitShadeContainer: ViewGroup? = null
-    private var useSplitShadeContainer: () -> Boolean = { false }
 
     /**
      * Attaches media container in single pane mode, situated at the top of the notifications list
      */
     fun attachSinglePaneContainer(mediaView: MediaHeaderView?) {
+        val needsListener = singlePaneContainer == null
         singlePaneContainer = mediaView
+        if (needsListener) {
+            // On reinflation we don't want to add another listener
+            mediaHost.addVisibilityChangeListener(this::onMediaHostVisibilityChanged)
+        }
+        reattachHostView()
+        onMediaHostVisibilityChanged(mediaHost.visible)
+    }
 
-        // Required to show it for the first time, afterwards visibility is managed automatically
+    /**
-        mediaHost.visible = true
+     * Called whenever the media hosts visibility changes
-        mediaHost.addVisibilityChangeListener { visible ->
+     */
-            refreshMediaPosition()
+    private fun onMediaHostVisibilityChanged(visible: Boolean) {
-            if (visible) {
+        refreshMediaPosition()
-                mediaHost.hostView.layoutParams.apply {
+        if (visible) {
-                    height = ViewGroup.LayoutParams.WRAP_CONTENT
+            mediaHost.hostView.layoutParams.apply {
-                    width = ViewGroup.LayoutParams.MATCH_PARENT
+                height = ViewGroup.LayoutParams.WRAP_CONTENT
-                }
+                width = ViewGroup.LayoutParams.MATCH_PARENT
             }
         }
-        refreshMediaPosition()
     }
 
     /**
      * Attaches media container in split shade mode, situated to the left of notifications
      */
-    fun attachSplitShadeContainer(container: ViewGroup, useContainer: () -> Boolean) {
+    fun attachSplitShadeContainer(container: ViewGroup) {
         splitShadeContainer = container
-        useSplitShadeContainer = useContainer
+        reattachHostView()
+        refreshMediaPosition()
+    }
+
+    private fun reattachHostView() {
+        val inactiveContainer: ViewGroup?
+        val activeContainer: ViewGroup?
+        if (useSplitShade) {
+            activeContainer = splitShadeContainer
+            inactiveContainer = singlePaneContainer
+        } else {
+            inactiveContainer = splitShadeContainer
+            activeContainer = singlePaneContainer
+        }
+        if (inactiveContainer?.childCount == 1) {
+            inactiveContainer.removeAllViews()
+        }
+        if (activeContainer?.childCount == 0) {
+            // Detach the hostView from its parent view if exists
+            mediaHost.hostView.parent?.let {
+                (it as? ViewGroup)?.removeView(mediaHost.hostView)
+            }
+            activeContainer.addView(mediaHost.hostView)
+        }
     }
 
     fun refreshMediaPosition() {
@@ -124,35 +176,17 @@ class KeyguardMediaController @Inject constructor(
     }
 
     private fun showMediaPlayer() {
-        if (useSplitShadeContainer()) {
+        if (useSplitShade) {
-            showMediaPlayer(
+            setVisibility(splitShadeContainer, View.VISIBLE)
-                    activeContainer = splitShadeContainer,
+            setVisibility(singlePaneContainer, View.GONE)
-                    inactiveContainer = singlePaneContainer)
         } else {
-            showMediaPlayer(
+            setVisibility(singlePaneContainer, View.VISIBLE)
-                    activeContainer = singlePaneContainer,
+            setVisibility(splitShadeContainer, View.GONE)
-                    inactiveContainer = splitShadeContainer)
         }
     }
 
-    private fun showMediaPlayer(activeContainer: ViewGroup?, inactiveContainer: ViewGroup?) {
-        if (inactiveContainer?.childCount == 1) {
-            inactiveContainer.removeAllViews()
-        }
-        // might be called a few times for the same view, no need to add hostView again
-        if (activeContainer?.childCount == 0) {
-            // Detach the hostView from its parent view if exists
-            mediaHost.hostView.parent ?.let {
-                (it as? ViewGroup)?.removeView(mediaHost.hostView)
-            }
-            activeContainer.addView(mediaHost.hostView)
-        }
-        setVisibility(activeContainer, View.VISIBLE)
-        setVisibility(inactiveContainer, View.GONE)
-    }
-
     private fun hideMediaPlayer() {
-        if (useSplitShadeContainer()) {
+        if (useSplitShade) {
             setVisibility(splitShadeContainer, View.GONE)
         } else {
             setVisibility(singlePaneContainer, View.GONE)

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

@@ -27,6 +27,11 @@ class MediaHost constructor(
 
     private var inited: Boolean = false
 
+    /**
+     * Are we listening to media data changes?
+     */
+    private var listeningToMediaData = false
+
     /**
      * Get the current bounds on the screen. This makes sure the state is fresh and up to date
      */
@@ -97,18 +102,17 @@ class MediaHost constructor(
 
         this.location = location
         hostView = mediaHierarchyManager.register(this)
+        // Listen by default, as the host might not be attached by our clients, until
+        // they get a visibility change. We still want to stay up to date in that case!
+        setListeningToMediaData(true)
         hostView.addOnAttachStateChangeListener(object : OnAttachStateChangeListener {
             override fun onViewAttachedToWindow(v: View?) {
-                // we should listen to the combined state change, since otherwise there might
+                setListeningToMediaData(true)
-                // be a delay until the views and the controllers are initialized, leaving us
-                // with either a blank view or the controllers not yet initialized and the
-                // measuring wrong
-                mediaDataManager.addListener(listener)
                 updateViewVisibility()
             }
 
             override fun onViewDetachedFromWindow(v: View?) {
-                mediaDataManager.removeListener(listener)
+                setListeningToMediaData(false)
             }
         })
 
@@ -135,8 +139,19 @@ class MediaHost constructor(
         updateViewVisibility()
     }
 
+    private fun setListeningToMediaData(listen: Boolean) {
+        if (listen != listeningToMediaData) {
+            listeningToMediaData = listen
+            if (listen) {
+                mediaDataManager.addListener(listener)
+            } else {
+                mediaDataManager.removeListener(listener)
+            }
+        }
+    }
+
     private fun updateViewVisibility() {
-        visible = if (showsOnlyActiveMedia) {
+        state.visible = if (showsOnlyActiveMedia) {
             mediaDataManager.hasActiveMedia()
         } else {
             mediaDataManager.hasAnyMedia()
@@ -300,7 +315,7 @@ interface MediaHostState {
     /**
      * If the view should be VISIBLE or GONE.
      */
-    var visible: Boolean
+    val visible: Boolean
 
     /**
      * Does this host need any falsing protection?

packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java

@@ -1085,8 +1085,7 @@ public class NotificationPanelViewController extends PanelViewController {
     }
 
     private void attachSplitShadeMediaPlayerContainer(FrameLayout container) {
-        mKeyguardMediaController.attachSplitShadeContainer(container,
+        mKeyguardMediaController.attachSplitShadeContainer(container);
-                () -> mShouldUseSplitNotificationShade);
     }
 
     private void initBottomArea() {

packages/SystemUI/tests/src/com/android/systemui/media/KeyguardMediaControllerTest.kt

@@ -22,13 +22,16 @@ import android.view.View.VISIBLE
 import android.widget.FrameLayout
 import androidx.test.filters.SmallTest
 import com.android.systemui.SysuiTestCase
+import com.android.systemui.statusbar.FeatureFlags
 import com.android.systemui.statusbar.NotificationLockscreenUserManager
 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.statusbar.policy.ConfigurationController
 import com.android.systemui.util.animation.UniqueObjectHostView
 import com.google.common.truth.Truth.assertThat
+import junit.framework.Assert.assertTrue
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -48,11 +51,16 @@ class KeyguardMediaControllerTest : SysuiTestCase() {
     @Mock
     private lateinit var statusBarStateController: SysuiStatusBarStateController
     @Mock
+    private lateinit var configurationController: ConfigurationController
+    @Mock
+    private lateinit var featureFlags: FeatureFlags
+    @Mock
     private lateinit var notificationLockscreenUserManager: NotificationLockscreenUserManager
     @JvmField @Rule
     val mockito = MockitoJUnit.rule()
 
     private val mediaHeaderView: MediaHeaderView = MediaHeaderView(context, null)
+    private val hostView = UniqueObjectHostView(context)
     private lateinit var keyguardMediaController: KeyguardMediaController
 
     @Before
@@ -62,10 +70,17 @@ class KeyguardMediaControllerTest : SysuiTestCase() {
         whenever(statusBarStateController.state).thenReturn(StatusBarState.KEYGUARD)
         whenever(notificationLockscreenUserManager.shouldShowLockscreenNotifications())
                 .thenReturn(true)
-        whenever(mediaHost.hostView).thenReturn(UniqueObjectHostView(context))
+        whenever(mediaHost.hostView).thenReturn(hostView)
 
-        keyguardMediaController = KeyguardMediaController(mediaHost, bypassController,
+        keyguardMediaController = KeyguardMediaController(
-                statusBarStateController, notificationLockscreenUserManager)
+            mediaHost,
+            bypassController,
+            statusBarStateController,
+            notificationLockscreenUserManager,
+            featureFlags,
+            context,
+            configurationController
+        )
         keyguardMediaController.attachSinglePaneContainer(mediaHeaderView)
     }
 
@@ -105,11 +120,8 @@ class KeyguardMediaControllerTest : SysuiTestCase() {
     @Test
     fun testActivatesSplitShadeContainerInSplitShadeMode() {
         val splitShadeContainer = FrameLayout(context)
-        keyguardMediaController.attachSplitShadeContainer(
+        keyguardMediaController.attachSplitShadeContainer(splitShadeContainer)
-                splitShadeContainer,
+        keyguardMediaController.useSplitShade = true
-                useContainer = { true })
-
-        keyguardMediaController.refreshMediaPosition()
 
         assertThat(splitShadeContainer.visibility).isEqualTo(VISIBLE)
     }
@@ -117,13 +129,28 @@ class KeyguardMediaControllerTest : SysuiTestCase() {
     @Test
     fun testActivatesSinglePaneContainerInSinglePaneMode() {
         val splitShadeContainer = FrameLayout(context)
-        keyguardMediaController.attachSplitShadeContainer(
+        keyguardMediaController.attachSplitShadeContainer(splitShadeContainer)
-                splitShadeContainer,
-                useContainer = { false })
-
-        keyguardMediaController.refreshMediaPosition()
 
         assertThat(splitShadeContainer.visibility).isEqualTo(GONE)
         assertThat(mediaHeaderView.visibility).isEqualTo(VISIBLE)
     }
+
+    @Test
+    fun testAttachedToSplitShade() {
+        val splitShadeContainer = FrameLayout(context)
+        keyguardMediaController.attachSplitShadeContainer(splitShadeContainer)
+        keyguardMediaController.useSplitShade = true
+
+        assertTrue("HostView wasn't attached to the split pane container",
+            splitShadeContainer.childCount == 1)
+    }
+
+    @Test
+    fun testAttachedToSinglePane() {
+        val splitShadeContainer = FrameLayout(context)
+        keyguardMediaController.attachSplitShadeContainer(splitShadeContainer)
+
+        assertTrue("HostView wasn't attached to the single pane container",
+            mediaHeaderView.childCount == 1)
+    }
 }