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

Commit ddfc6fd8 authored by Olivier St-Onge's avatar Olivier St-Onge
Browse files

Fix concurrency error when resetting tiles and sizes at the same time 

DynamicIconTilesInteractor doesn't need to listen to the largeTiles flow when removing deleted tiles. Instead pass the removed tiles to QSPreferencesRepository and let it update the set accordingly

Added a test case that repros this issue in DynamicIconTilesInteractorTest

Flag: com.android.systemui.qs_ui_refactor_compose_fragment
Fixes: 411526683
Test: QSPreferencesRepositoryTest.kt
Test: DynamicIconTilesInteractorTest.kt
Change-Id: I8673bb9227909679ef91a95f44d9bd2211f1a70c
parent 465c4874

packages/SystemUI/multivalentTests/src/com/android/systemui/qs/panels/data/repository/QSPreferencesRepositoryTest.kt

+9 −0
Original line number Diff line number Diff line
@@ -85,6 +85,15 @@ class QSPreferencesRepositoryTest : SysuiTestCase() { 
        assertThat(getLargeTilesSpecsFromSharedPreferences()).isEqualTo(setB)

    }



    @Test

    fun removeLargeTilesSpecs_inSharedPreferences() {

        val setA = setOf("tileA", "tileB", "tileC", "tileD")

        setLargeTilesSpecsInSharedPreferences(setA)



        underTest.removeLargeTileSpecs(setOf(TileSpec.create("tileB"), TileSpec.create("tileD")))

        assertThat(getLargeTilesSpecsFromSharedPreferences()).isEqualTo(setOf("tileA", "tileC"))

    }



    @Test

    fun setLargeTilesSpecs_forDifferentUser() =

        with(kosmos) {

packages/SystemUI/multivalentTests/src/com/android/systemui/qs/panels/domain/interactor/DynamicIconTilesInteractorTest.kt

+21 −0
Original line number Diff line number Diff line
@@ -29,12 +29,14 @@ import com.android.systemui.qs.pipeline.domain.interactor.currentTilesInteractor 
import com.android.systemui.qs.pipeline.shared.TileSpec

import com.android.systemui.testKosmos

import com.google.common.truth.Truth.assertThat

import kotlinx.coroutines.ExperimentalCoroutinesApi

import kotlinx.coroutines.test.runCurrent

import kotlinx.coroutines.test.runTest

import org.junit.Before

import org.junit.Test

import org.junit.runner.RunWith



@OptIn(ExperimentalCoroutinesApi::class)

@SmallTest

@RunWith(AndroidJUnit4::class)

class DynamicIconTilesInteractorTest : SysuiTestCase() {
@@ -72,8 +74,27 @@ class DynamicIconTilesInteractorTest : SysuiTestCase() { 
            }

        }



    @Test

    fun removingAndResizingTiles_updatesSharedPreferences() =

        with(kosmos) {

            testScope.runTest {

                val latest by collectLastValue(qsPreferencesRepository.largeTilesSpecs)

                runCurrent()



                // Remove the large tile from the current tiles

                iconTilesInteractor.setLargeTiles(latest!! + newTile)

                currentTilesInteractor.removeTiles(listOf(largeTile))

                runCurrent()



                // Assert that it resized to small

                assertThat(latest).doesNotContain(largeTile)

                assertThat(latest).contains(newTile)

            }

        }



    private companion object {

        private val largeTile = TileSpec.create("large")

        private val smallTile = TileSpec.create("small")

        private val newTile = TileSpec.create("new")

    }

}

packages/SystemUI/src/com/android/systemui/qs/panels/data/repository/QSPreferencesRepository.kt

+19 −9
Original line number Diff line number Diff line
@@ -71,15 +71,7 @@ constructor( 
        combine(backupRestorationEvents, userRepository.selectedUserInfo, ::Pair)

            .flatMapLatest { (_, userInfo) ->

                val prefs = getSharedPrefs(userInfo.id)

                prefs.observe().emitOnStart().map {

                    prefs

                        .getStringSet(

                            LARGE_TILES_SPECS_KEY,

                            defaultLargeTilesRepository.defaultLargeTiles.map { it.spec }.toSet(),

                        )

                        ?.map { TileSpec.create(it) }

                        ?.toSet() ?: defaultLargeTilesRepository.defaultLargeTiles

                }

                prefs.observe().emitOnStart().map { prefs.getLargeTilesSpecs() }

            }

            .flowOn(backgroundDispatcher)
@@ -91,6 +83,15 @@ constructor( 
        }

    }



    /** Remove the set of [TileSpec] from the current large tiles. */

    fun removeLargeTileSpecs(specs: Set<TileSpec>) {

        with(getSharedPrefs(userRepository.getSelectedUserInfo().id)) {

            val largeSpecs = getLargeTilesSpecs()

            writeLargeTileSpecs(largeSpecs - specs)

            setLargeTilesDefault(false)

        }

    }



    suspend fun deleteLargeTileDataJob() {

        userRepository.selectedUserInfo.collect { userInfo ->

            getSharedPrefs(userInfo.id)
@@ -105,6 +106,15 @@ constructor( 
        edit().putStringSet(LARGE_TILES_SPECS_KEY, specs.map { it.spec }.toSet()).apply()

    }



    private fun SharedPreferences.getLargeTilesSpecs(): Set<TileSpec> {

        return getStringSet(

                LARGE_TILES_SPECS_KEY,

                defaultLargeTilesRepository.defaultLargeTiles.map { it.spec }.toSet(),

            )

            ?.map { TileSpec.create(it) }

            ?.toSet() ?: defaultLargeTilesRepository.defaultLargeTiles

    }



    /**

     * Sets the initial set of large tiles. One of the following cases will happen:

     * * If we are setting the default set (no value stored in settings for the list of tiles), set

packages/SystemUI/src/com/android/systemui/qs/panels/domain/interactor/DynamicIconTilesInteractor.kt

+36 −8
Original line number Diff line number Diff line
@@ -17,9 +17,18 @@ 
package com.android.systemui.qs.panels.domain.interactor



import com.android.systemui.lifecycle.ExclusiveActivatable

import com.android.systemui.log.LogBuffer

import com.android.systemui.log.core.LogLevel

import com.android.systemui.qs.panels.shared.model.PanelsLog

import com.android.systemui.qs.pipeline.domain.interactor.CurrentTilesInteractor

import com.android.systemui.qs.pipeline.shared.TileSpec

import com.android.systemui.util.kotlin.pairwise

import dagger.assisted.AssistedFactory

import dagger.assisted.AssistedInject

import kotlinx.coroutines.awaitCancellation

import kotlinx.coroutines.flow.filter

import kotlinx.coroutines.flow.map

import kotlinx.coroutines.flow.onEach



/** Interactor to resize QS tiles down to icons when removed from the current tiles. */

class DynamicIconTilesInteractor
@@ -27,21 +36,40 @@ class DynamicIconTilesInteractor 
constructor(

    private val iconTilesInteractor: IconTilesInteractor,

    private val currentTilesInteractor: CurrentTilesInteractor,

    @PanelsLog private val logBuffer: LogBuffer,

) : ExclusiveActivatable() {



    override suspend fun onActivated(): Nothing {

        currentTilesInteractor.currentTiles.collect { currentTiles ->

            // Only current tiles can be resized, so observe the current tiles and find the

            // intersection between them and the large tiles.

            val newLargeTiles =

                iconTilesInteractor.largeTilesSpecs.value intersect

                    currentTiles.map { it.spec }.toSet()

            iconTilesInteractor.setLargeTiles(newLargeTiles)

        currentTilesInteractor.userAndTiles

            .pairwise()

            .filter { !it.newValue.userChange } // Only compare tile changes for the same user

            .map { tilesData ->

                // Only current tiles can be resized, so find removed tiles before updating their

                // sizes

                val removedTiles = tilesData.previousValue.tiles - tilesData.newValue.tiles.toSet()

                removedTiles.toSet()

            }

            .filter { it.isNotEmpty() }

            .onEach { logChange(it) }

            .collect { removedTiles -> iconTilesInteractor.removeLargeTiles(removedTiles) }

        awaitCancellation()

    }



    @AssistedFactory

    interface Factory {

        fun create(): DynamicIconTilesInteractor

    }



    private fun logChange(deletedSpecs: Set<TileSpec>) {

        logBuffer.log(

            LOG_BUFFER_DELETED_TILES_RESIZED_TAG,

            LogLevel.DEBUG,

            { str1 = deletedSpecs.toString() },

            { "Removed tiles=$str1" },

        )

    }



    private companion object {

        const val LOG_BUFFER_DELETED_TILES_RESIZED_TAG = "RemovedTiles"

    }

}

packages/SystemUI/src/com/android/systemui/qs/panels/domain/interactor/IconTilesInteractor.kt

+6 −0
Original line number Diff line number Diff line
@@ -58,10 +58,16 @@ constructor( 


    fun isIconTile(spec: TileSpec): Boolean = !largeTilesSpecs.value.contains(spec)



    /** Set the large tiles to be [specs] */

    fun setLargeTiles(specs: Set<TileSpec>) {

        preferencesInteractor.setLargeTilesSpecs(specs)

    }



    /** Remove [specs] from the current set of large tiles */

    fun removeLargeTiles(specs: Set<TileSpec>) {

        preferencesInteractor.removeLargeTilesSpecs(specs)

    }



    fun resetToDefault() {

        preferencesInteractor.setLargeTilesSpecs(repo.defaultLargeTiles)

    }