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

Commit f62492df authored by András Kurucz's avatar András Kurucz
Browse files

Cleanup in BigPictureIconManager

- Make sure that displayedState is in sync with the drawable that we've
set on the consumer.
- User clearInvocations in the tests instead of resetting the mocks.

Bug: 283082473
Test: atest BigPictureIconManager
Change-Id: I94571e4caeb8302c5aa836f01f8ac2cd1563c7c2
parent 54975542
Loading
Loading
Loading
Loading
+30 −22
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import android.graphics.drawable.Icon
import android.util.Dumpable
import android.util.Log
import android.util.Size
import androidx.annotation.MainThread
import com.android.internal.R
import com.android.internal.widget.NotificationDrawableConsumer
import com.android.internal.widget.NotificationIconManager
@@ -33,7 +34,9 @@ import com.android.systemui.dagger.qualifiers.Main
import com.android.systemui.graphics.ImageLoader
import com.android.systemui.statusbar.notification.row.BigPictureIconManager.DrawableState.Empty
import com.android.systemui.statusbar.notification.row.BigPictureIconManager.DrawableState.FullImage
import com.android.systemui.statusbar.notification.row.BigPictureIconManager.DrawableState.Initial
import com.android.systemui.statusbar.notification.row.BigPictureIconManager.DrawableState.PlaceHolder
import com.android.systemui.util.Assert
import java.io.PrintWriter
import javax.inject.Inject
import kotlin.math.min
@@ -67,7 +70,7 @@ constructor(

    private var lastLoadingJob: Job? = null
    private var drawableConsumer: NotificationDrawableConsumer? = null
    private var displayedState: DrawableState = Empty(null)
    private var displayedState: DrawableState = Initial
    private var viewShown = false

    private var maxWidth = getMaxWidth()
@@ -90,7 +93,6 @@ constructor(
            this.lastLoadingJob =
                when {
                    skipLazyLoading(state.icon) -> null
                    state is Empty && shown -> state.icon?.let(::startLoadingJob)
                    state is PlaceHolder && shown -> startLoadingJob(state.icon)
                    state is FullImage && !shown ->
                        startFreeImageJob(state.icon, state.drawableSize)
@@ -121,14 +123,12 @@ constructor(
        }

        this.drawableConsumer = drawableConsumer
        this.displayedState = Empty(icon)
        this.lastLoadingJob?.cancel()

        val drawable = loadImageOrPlaceHolderSync(icon)

        val drawableAndState = loadImageOrPlaceHolderSync(icon)
        log("icon updated")

        return Runnable { drawableConsumer.setImageDrawable(drawable) }
        return Runnable { applyDrawableAndState(drawableAndState) }
    }

    override fun dump(pw: PrintWriter, args: Array<out String>?) {
@@ -136,7 +136,7 @@ constructor(
    }

    @WorkerThread
    private fun loadImageOrPlaceHolderSync(icon: Icon?): Drawable? {
    private fun loadImageOrPlaceHolderSync(icon: Icon?): Pair<Drawable, DrawableState>? {
        icon ?: return null

        if (viewShown || skipLazyLoading(icon)) {
@@ -147,10 +147,10 @@ constructor(
    }

    @WorkerThread
    private fun loadImageSync(icon: Icon): Drawable? {
        return imageLoader.loadDrawableSync(icon, context, maxWidth, maxHeight)?.also { drawable ->
    private fun loadImageSync(icon: Icon): Pair<Drawable, DrawableState>? {
        return imageLoader.loadDrawableSync(icon, context, maxWidth, maxHeight)?.let { drawable ->
            checkPlaceHolderSizeForDrawable(this.displayedState, drawable)
            this.displayedState = FullImage(icon, drawable.intrinsicSize)
            Pair(drawable, FullImage(icon, drawable.intrinsicSize))
        }
    }

@@ -173,32 +173,39 @@ constructor(
    }

    @WorkerThread
    private fun loadPlaceHolderSync(icon: Icon): Drawable? {
    private fun loadPlaceHolderSync(icon: Icon): Pair<Drawable, DrawableState>? {
        return imageLoader
            .loadSizeSync(icon, context)
            ?.resizeToMax(maxWidth, maxHeight) // match the dimensions of the fully loaded drawable
            ?.let { size -> createPlaceHolder(size) }
            ?.also { drawable -> this.displayedState = PlaceHolder(icon, drawable.intrinsicSize) }
            ?.let { size -> createPlaceHolder(icon, size) }
    }

    @MainThread
    private fun applyDrawableAndState(drawableAndState: Pair<Drawable, DrawableState>?) {
        Assert.isMainThread()
        drawableConsumer?.setImageDrawable(drawableAndState?.first)
        displayedState = drawableAndState?.second ?: Empty
    }

    private fun startLoadingJob(icon: Icon): Job =
        scope.launch {
            val drawable = withContext(bgDispatcher) { loadImageSync(icon) }
            withContext(mainDispatcher) { drawableConsumer?.setImageDrawable(drawable) }
            log("image loaded")
            val drawableAndState = withContext(bgDispatcher) { loadImageSync(icon) }
            withContext(mainDispatcher) { applyDrawableAndState(drawableAndState) }
            log("full image loaded")
        }

    private fun startFreeImageJob(icon: Icon, drawableSize: Size): Job =
        scope.launch {
            delay(FREE_IMAGE_DELAY_MS)
            val drawable = createPlaceHolder(drawableSize)
            displayedState = PlaceHolder(icon, drawable.intrinsicSize)
            withContext(mainDispatcher) { drawableConsumer?.setImageDrawable(drawable) }
            val drawableAndState = createPlaceHolder(icon, drawableSize)
            withContext(mainDispatcher) { applyDrawableAndState(drawableAndState) }
            log("placeholder loaded")
        }

    private fun createPlaceHolder(size: Size): Drawable {
        return PlaceHolderDrawable(width = size.width, height = size.height)
    private fun createPlaceHolder(icon: Icon, size: Size): Pair<Drawable, DrawableState> {
        val drawable = PlaceHolderDrawable(width = size.width, height = size.height)
        val state = PlaceHolder(icon, drawable.intrinsicSize)
        return Pair(drawable, state)
    }

    private fun isLowRam(): Boolean {
@@ -246,7 +253,8 @@ constructor(
        "{ state:$displayedState, hasConsumer:${drawableConsumer != null}, viewShown:$viewShown}"

    private sealed class DrawableState(open val icon: Icon?) {
        data class Empty(override val icon: Icon?) : DrawableState(icon)
        data object Initial : DrawableState(null)
        data object Empty : DrawableState(null)
        data class PlaceHolder(override val icon: Icon, val drawableSize: Size) :
            DrawableState(icon)
        data class FullImage(override val icon: Icon, val drawableSize: Size) : DrawableState(icon)
+24 −15
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import android.graphics.drawable.Drawable
import android.graphics.drawable.Icon
import android.net.Uri
import android.testing.AndroidTestingRunner
import android.testing.TestableLooper.RunWithLooper
import androidx.test.filters.SmallTest
import com.android.internal.widget.NotificationDrawableConsumer
import com.android.systemui.res.R
@@ -35,10 +36,11 @@ import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.advanceTimeBy
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import org.junit.After
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.reset
import org.mockito.Mockito.clearInvocations
import org.mockito.Mockito.verify
import org.mockito.Mockito.verifyZeroInteractions

@@ -46,6 +48,7 @@ private const val FREE_IMAGE_DELAY_MS = 4000L

@OptIn(ExperimentalCoroutinesApi::class)
@SmallTest
@RunWithLooper
@RunWith(AndroidTestingRunner::class)
class BigPictureIconManagerTest : SysuiTestCase() {

@@ -77,6 +80,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {

    @Before
    fun setUp() {
        allowTestableLooperAsMainThread()
        iconManager =
            BigPictureIconManager(
                context,
@@ -87,6 +91,11 @@ class BigPictureIconManagerTest : SysuiTestCase() {
            )
    }

    @After
    fun tearDown() {
        disallowTestableLooperAsMainThread()
    }

    @Test
    fun onIconUpdated_supportedType_placeholderLoaded() =
        testScope.runTest {
@@ -137,7 +146,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
            // GIVEN a consumer is set
            val otherConsumer: NotificationDrawableConsumer = mock()
            iconManager.updateIcon(mockConsumer, supportedIcon).run()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN a new consumer is set
            iconManager.updateIcon(otherConsumer, unsupportedIcon).run()
@@ -151,7 +160,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
        testScope.runTest {
            // GIVEN an icon is set
            iconManager.updateIcon(mockConsumer, supportedIcon).run()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN a new icon is set
            iconManager.updateIcon(mockConsumer, unsupportedIcon).run()
@@ -170,7 +179,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
            // AND the view is shown
            iconManager.onViewShown(true)
            runCurrent()
            reset(mockConsumer)
            clearInvocations(mockConsumer)
            // WHEN the icon is set again
            iconManager.updateIcon(mockConsumer, supportedIcon).run()

@@ -188,7 +197,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
            // AND the view is shown
            iconManager.onViewShown(true)
            runCurrent()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN a new icon is set
            iconManager.updateIcon(mockConsumer, supportedIcon).run()
@@ -242,7 +251,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
        testScope.runTest {
            // GIVEN placeholder is showing
            iconManager.updateIcon(mockConsumer, supportedIcon).run()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN the view is shown
            iconManager.onViewShown(true)
@@ -261,7 +270,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
            iconManager.updateIcon(mockConsumer, supportedIcon).run()
            iconManager.onViewShown(true)
            runCurrent()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN the view goes off the screen
            iconManager.onViewShown(false)
@@ -282,7 +291,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
            iconManager.updateIcon(mockConsumer, supportedIcon).run()
            iconManager.onViewShown(true)
            runCurrent()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN the onViewShown is toggled
            iconManager.onViewShown(false)
@@ -301,7 +310,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
        testScope.runTest {
            // GIVEN full image is showing for an unsupported icon
            iconManager.updateIcon(mockConsumer, unsupportedIcon).run()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN the view is shown
            iconManager.onViewShown(true)
@@ -316,7 +325,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
        testScope.runTest {
            // GIVEN null is set for the icon
            iconManager.updateIcon(mockConsumer, null).run()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN the view is shown
            iconManager.onViewShown(true)
@@ -334,7 +343,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
            // AND the view is shown
            iconManager.onViewShown(true)
            runCurrent()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN the view goes off the screen
            iconManager.onViewShown(false)
@@ -351,7 +360,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
        testScope.runTest {
            // GIVEN placeholder image is showing
            iconManager.updateIcon(mockConsumer, supportedIcon).run()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN the view is hidden
            iconManager.onViewShown(false)
@@ -370,7 +379,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
            iconManager.updateIcon(mockConsumer, supportedIcon).run()
            iconManager.onViewShown(true)
            runCurrent()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN view shown called again
            iconManager.onViewShown(true)
@@ -388,7 +397,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
            iconManager.onViewShown(false)
            advanceTimeBy(FREE_IMAGE_DELAY_MS)
            runCurrent()
            reset(mockConsumer)
            clearInvocations(mockConsumer)

            // WHEN the view is hidden again
            iconManager.onViewShown(false)
@@ -407,7 +416,7 @@ class BigPictureIconManagerTest : SysuiTestCase() {
            iconManager.updateIcon(mockConsumer, supportedIcon).run()
            iconManager.onViewShown(true)
            runCurrent()
            reset(mockConsumer)
            clearInvocations(mockConsumer)
            // AND the view has just gone off the screen
            iconManager.onViewShown(false)