Merge "Fix race condition in TileSpecRepository" into main (7eb15d04) · Commits · e / os / android_frameworks_base

packages/SystemUI/src/com/android/systemui/qs/TEST_MAPPING

+11 −0

Original line number	Diff line number	Diff line
		@@ -10,6 +10,17 @@
		"exclude-annotation": "androidx.test.filters.FlakyTest"
		}
		]
		},
		{
		"name": "QuickSettingsDeviceResetTests",
		"options": [
		{
		"exclude-annotation": "org.junit.Ignore"
		},
		{
		"exclude-annotation": "androidx.test.filters.FlakyTest"
		}
		]
		}
		]
		}
		No newline at end of file

packages/SystemUI/src/com/android/systemui/qs/pipeline/data/repository/TileSpecRepository.kt

+18 −14

Original line number	Diff line number	Diff line
		@@ -20,6 +20,7 @@ import android.annotation.UserIdInt
		import android.content.res.Resources
		import android.database.ContentObserver
		import android.provider.Settings
		import android.util.SparseArray
		import com.android.systemui.R
		import com.android.systemui.common.coroutine.ConflatedCallbackFlow.conflatedCallbackFlow
		import com.android.systemui.dagger.SysUISingleton
		@@ -107,6 +108,7 @@ constructor(
		) : TileSpecRepository {

		private val mutex = Mutex()
		private val tileSpecsPerUser = SparseArray<List<TileSpec>>()

		private val retailModeTiles by lazy {
		resources
		@@ -142,10 +144,12 @@ constructor(
		awaitClose { secureSettings.unregisterContentObserver(observer) }
		}
		.onStart { emit(Unit) }
		.map { secureSettings.getStringForUser(SETTING, userId) ?: "" }
		.distinctUntilChanged()
		.map { loadTiles(userId) }
		.onEach { logger.logTilesChangedInSettings(it, userId) }
		.map { parseTileSpecs(it, userId) }
		.distinctUntilChanged()
		.map { parseTileSpecs(it, userId).also { storeTiles(userId, it) } }
		.distinctUntilChanged()
		.onEach { mutex.withLock { tileSpecsPerUser.put(userId, it) } }
		.flowOn(backgroundDispatcher)
		}

		@@ -154,7 +158,7 @@ constructor(
		if (tile == TileSpec.Invalid) {
		return
		}
		val tilesList = loadTiles(userId).toMutableList()
		val tilesList = tileSpecsPerUser.get(userId, emptyList()).toMutableList()
		if (tile !in tilesList) {
		if (position < 0 \|\| position >= tilesList.size) {
		tilesList.add(tile)
		@@ -162,6 +166,7 @@ constructor(
		tilesList.add(position, tile)
		}
		storeTiles(userId, tilesList)
		tileSpecsPerUser.put(userId, tilesList)
		}
		}

		@@ -170,9 +175,10 @@ constructor(
		if (tiles.all { it == TileSpec.Invalid }) {
		return
		}
		val tilesList = loadTiles(userId).toMutableList()
		val tilesList = tileSpecsPerUser.get(userId, emptyList()).toMutableList()
		if (tilesList.removeAll(tiles)) {
		storeTiles(userId, tilesList.toList())
		tileSpecsPerUser.put(userId, tilesList)
		}
		}

		@@ -181,15 +187,7 @@ constructor(
		val filtered = tiles.filter { it != TileSpec.Invalid }
		if (filtered.isNotEmpty()) {
		storeTiles(userId, filtered)
		}
		}

		private suspend fun loadTiles(@UserIdInt forUser: Int): List<TileSpec> {
		return withContext(backgroundDispatcher) {
		(secureSettings.getStringForUser(SETTING, forUser) ?: "")
		.split(DELIMITER)
		.map(TileSpec::create)
		.filter { it !is TileSpec.Invalid }
		tileSpecsPerUser.put(userId, tiles)
		}
		}

		@@ -214,6 +212,12 @@ constructor(
		}
		}

		private suspend fun loadTiles(userId: Int): String {
		return withContext(backgroundDispatcher) {
		secureSettings.getStringForUser(SETTING, userId) ?: ""
		}
		}

		private fun parseTileSpecs(tilesFromSettings: String, user: Int): List<TileSpec> {
		val fromSettings =
		tilesFromSettings.split(DELIMITER).map(TileSpec::create).filter {

packages/SystemUI/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractor.kt

+5 −1

Original line number	Diff line number	Diff line
		@@ -56,6 +56,8 @@ import kotlinx.coroutines.flow.asStateFlow
		import kotlinx.coroutines.flow.collectLatest
		import kotlinx.coroutines.flow.combine
		import kotlinx.coroutines.flow.distinctUntilChanged
		import kotlinx.coroutines.flow.filter
		import kotlinx.coroutines.flow.first
		import kotlinx.coroutines.flow.flatMapLatest
		import kotlinx.coroutines.flow.flowOn
		import kotlinx.coroutines.flow.map
		@@ -273,7 +275,9 @@ constructor(
		}

		override fun addTile(spec: TileSpec, position: Int) {
		scope.launch {
		scope.launch(backgroundDispatcher) {
		// Block until the list is not empty
		currentTiles.filter { it.isNotEmpty() }.first()
		tileSpecRepository.addTile(userRepository.getSelectedUserInfo().id, spec, position)
		}
		}

packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/data/repository/TileSpecSettingsRepositoryTest.kt

+13 −1

Original line number	Diff line number	Diff line
		@@ -32,6 +32,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
		import kotlinx.coroutines.coroutineScope
		import kotlinx.coroutines.test.StandardTestDispatcher
		import kotlinx.coroutines.test.TestScope
		import kotlinx.coroutines.test.runCurrent
		import kotlinx.coroutines.test.runTest
		import org.junit.Before
		import org.junit.Test
		@@ -372,12 +373,23 @@ class TileSpecSettingsRepositoryTest : SysuiTestCase() {
		assertThat(loadTilesForUser(0)).isNull()
		}

		@Test
		fun emptyTilesReplacedByDefaultInSettings() =
		testScope.runTest {
		val tiles by collectLastValue(underTest.tilesSpecs(0))
		runCurrent()

		assertThat(loadTilesForUser(0))
		.isEqualTo(getDefaultTileSpecs().map { it.spec }.joinToString(","))
		}

		private fun getDefaultTileSpecs(): List<TileSpec> {
		return QSHost.getDefaultSpecs(context.resources).map(TileSpec::create)
		}

		private fun storeTilesForUser(specs: String, forUser: Int) {
		private fun TestScope.storeTilesForUser(specs: String, forUser: Int) {
		secureSettings.putStringForUser(SETTING, specs, forUser)
		runCurrent()
		}

		private fun loadTilesForUser(forUser: Int): String? {

packages/SystemUI/tests/src/com/android/systemui/qs/pipeline/domain/interactor/CurrentTilesInteractorImplTest.kt

+16 −0

Original line number	Diff line number	Diff line
		@@ -648,6 +648,22 @@ class CurrentTilesInteractorImplTest : SysuiTestCase() {
		assertThat(tiles!![1].spec).isEqualTo(CUSTOM_TILE_SPEC)
		}

		@Test
		fun tileAddedOnEmptyList_blocked() =
		testScope.runTest(USER_INFO_0) {
		val tiles by collectLastValue(underTest.currentTiles)
		val specs = listOf(TileSpec.create("a"), TileSpec.create("b"))
		val newTile = TileSpec.create("c")

		underTest.addTile(newTile)

		assertThat(tiles!!.isEmpty()).isTrue()

		tileSpecRepository.setTiles(USER_INFO_0.id, specs)

		assertThat(tiles!!.size).isEqualTo(3)
		}

		private fun QSTile.State.fillIn(state: Int, label: CharSequence, secondaryLabel: CharSequence) {
		this.state = state
		this.label = label