Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e9587bb5 authored by Selim Cinek's avatar Selim Cinek
Browse files

Fixed an issue where the keyguard media visibility was wrong 

Because we were overriding the visibility to avoid some bugs,
this accidentally had the effect that hidden media players would
become visible. These visibility overrides were masking an
issue where the hostview wasn't attached to neither of the
containers, leading to the host not receiving any updates.
The mediaHost was waiting until being attached to start
listening, which is now also changed to avoid similar
future issues.

Fixes: 188530838
Test: atest SystemUITests
Change-Id: I8f5aad143713c057cec068f42b5f68cffc370861
parent 48f51861

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

+78 −44
Original line number Diff line number Diff line
@@ -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) {

                if (!isDozing) {

                    mediaHost.visible = true

                    refreshMediaPosition()

                }

        })

        configurationController.addCallback(object : ConfigurationController.ConfigurationListener {

            override fun onConfigChanged(newConfig: Configuration?) {

                updateResources()

            }

        })
@@ -62,6 +69,22 @@ 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()

        }



    /**
@@ -78,17 +101,25 @@ 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

        mediaHost.addVisibilityChangeListener { visible ->

    /**

     * Called whenever the media hosts visibility changes

     */

    private fun onMediaHostVisibilityChanged(visible: Boolean) {

        refreshMediaPosition()

        if (visible) {

            mediaHost.hostView.layoutParams.apply {
@@ -97,15 +128,36 @@ class KeyguardMediaController @Inject constructor( 
            }

        }

    }

        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()) {

            showMediaPlayer(

                    activeContainer = splitShadeContainer,

                    inactiveContainer = singlePaneContainer)

        if (useSplitShade) {

            setVisibility(splitShadeContainer, View.VISIBLE)

            setVisibility(singlePaneContainer, View.GONE)

        } else {

            showMediaPlayer(

                    activeContainer = singlePaneContainer,

                    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(singlePaneContainer, View.VISIBLE)

            setVisibility(splitShadeContainer, View.GONE)

        }

        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

+23 −8
Original line number Diff line number Diff line
@@ -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

                // 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)

                setListeningToMediaData(true)

                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

+1 −2
Original line number Diff line number Diff line
@@ -1085,8 +1085,7 @@ public class NotificationPanelViewController extends PanelViewController { 
    }



    private void attachSplitShadeMediaPlayerContainer(FrameLayout container) {

        mKeyguardMediaController.attachSplitShadeContainer(container,

                () -> mShouldUseSplitNotificationShade);

        mKeyguardMediaController.attachSplitShadeContainer(container);

    }



    private void initBottomArea() {

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

+41 −14
Original line number Diff line number Diff line
@@ -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))



        keyguardMediaController = KeyguardMediaController(mediaHost, bypassController,

                statusBarStateController, notificationLockscreenUserManager)

        whenever(mediaHost.hostView).thenReturn(hostView)



        keyguardMediaController = KeyguardMediaController(

            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(

                splitShadeContainer,

                useContainer = { true })



        keyguardMediaController.refreshMediaPosition()

        keyguardMediaController.attachSplitShadeContainer(splitShadeContainer)

        keyguardMediaController.useSplitShade = true



        assertThat(splitShadeContainer.visibility).isEqualTo(VISIBLE)

    }
@@ -117,13 +129,28 @@ class KeyguardMediaControllerTest : SysuiTestCase() { 
    @Test

    fun testActivatesSinglePaneContainerInSinglePaneMode() {

        val splitShadeContainer = FrameLayout(context)

        keyguardMediaController.attachSplitShadeContainer(

                splitShadeContainer,

                useContainer = { false })



        keyguardMediaController.refreshMediaPosition()

        keyguardMediaController.attachSplitShadeContainer(splitShadeContainer)



        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)

    }

}