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

Commit 4fbbb7c6 authored by Fabián Kozynski's avatar Fabián Kozynski
Browse files

Prevent tiles in bad state by interrupted collectLatest

If the collectLatest in CurrentTilesInteractor is restarted, the
individual tiles may end up in a bad state (destroyed but not removed)
or they may not have gotten the new user, but now we are not in a user
change scenario.

Instead, do the following:
* Change the user if the tile is in the wrong user
* Filter out destroyed tiles

Test: atest com.android.systemui.qs
Fixes: 394715298
Flag: EXEMPT bugfix
Change-Id: Iff6acb913519c786dc5e6d47c0c6ff4eab1f0d8a
parent d816d4bb
Loading
Loading
Loading
Loading
+10 −0
Original line number Original line Diff line number Diff line
@@ -423,5 +423,15 @@ public class TileQueryHelperTest extends SysuiTestCase {


        @Override
        @Override
        public void destroy() {}
        public void destroy() {}

        @Override
        public boolean isDestroyed() {
            return false;
        }

        @Override
        public int getCurrentTileUser() {
            return 0;
        }
    }
    }
}
}
+126 −114
Original line number Original line Diff line number Diff line
@@ -56,20 +56,27 @@ class TilesAvailabilityInteractorTest(flags: FlagsParameterization) : SysuiTestC


    private val createdTiles = mutableListOf<FakeQSTile>()
    private val createdTiles = mutableListOf<FakeQSTile>()


    private val kosmos = testKosmos().apply {
    private val kosmos =
        testKosmos().apply {
            tileAvailabilityInteractorsMap = buildMap {
            tileAvailabilityInteractorsMap = buildMap {
                put(AIRPLANE_MODE_TILE_SPEC, QSTileAvailabilityInteractor.AlwaysAvailableInteractor)
                put(AIRPLANE_MODE_TILE_SPEC, QSTileAvailabilityInteractor.AlwaysAvailableInteractor)
            put(WORK_MODE_TILE_SPEC, FakeTileAvailabilityInteractor(
                put(
                    mapOf(
                    WORK_MODE_TILE_SPEC,
                            fakeUserRepository.getSelectedUserInfo().id to flowOf(true),
                    FakeTileAvailabilityInteractor(
                    ).withDefault { flowOf(false) }
                        mapOf(fakeUserRepository.getSelectedUserInfo().id to flowOf(true))
            ))
                            .withDefault { flowOf(false) }
            put(HOTSPOT_TILE_SPEC, FakeTileAvailabilityInteractor(
                    ),
                )
                put(
                    HOTSPOT_TILE_SPEC,
                    FakeTileAvailabilityInteractor(
                        emptyMap<Int, Flow<Boolean>>().withDefault { flowOf(false) }
                        emptyMap<Int, Flow<Boolean>>().withDefault { flowOf(false) }
            ))
                    ),
                )
            }
            }


        qsTileFactory = constantFactory(
            qsTileFactory =
                constantFactory(
                    tilesForCreator(
                    tilesForCreator(
                        userRepository.getSelectedUserInfo().id,
                        userRepository.getSelectedUserInfo().id,
                        mapOf(
                        mapOf(
@@ -78,7 +85,7 @@ class TilesAvailabilityInteractorTest(flags: FlagsParameterization) : SysuiTestC
                            HOTSPOT_TILE_SPEC to true,
                            HOTSPOT_TILE_SPEC to true,
                            INTERNET_TILE_SPEC to true,
                            INTERNET_TILE_SPEC to true,
                            FLASHLIGHT_TILE_SPEC to false,
                            FLASHLIGHT_TILE_SPEC to false,
                        )
                        ),
                    )
                    )
                )
                )
        }
        }
@@ -87,83 +94,91 @@ class TilesAvailabilityInteractorTest(flags: FlagsParameterization) : SysuiTestC


    @Test
    @Test
    @DisableFlags(FLAG_QS_NEW_TILES)
    @DisableFlags(FLAG_QS_NEW_TILES)
    fun flagOff_usesAvailabilityFromFactoryTiles() = with(kosmos) {
    fun flagOff_usesAvailabilityFromFactoryTiles() =
        with(kosmos) {
            testScope.runTest {
            testScope.runTest {
            val unavailableTiles = underTest.getUnavailableTiles(
                val unavailableTiles =
                    underTest.getUnavailableTiles(
                        setOf(
                        setOf(
                                AIRPLANE_MODE_TILE_SPEC,
                                AIRPLANE_MODE_TILE_SPEC,
                                WORK_MODE_TILE_SPEC,
                                WORK_MODE_TILE_SPEC,
                                HOTSPOT_TILE_SPEC,
                                HOTSPOT_TILE_SPEC,
                                INTERNET_TILE_SPEC,
                                INTERNET_TILE_SPEC,
                                FLASHLIGHT_TILE_SPEC,
                                FLASHLIGHT_TILE_SPEC,
                    ).map(TileSpec::create)
                            )
                            )
            assertThat(unavailableTiles).isEqualTo(setOf(
                            .map(TileSpec::create)
                    AIRPLANE_MODE_TILE_SPEC,
                    )
                    WORK_MODE_TILE_SPEC,
                assertThat(unavailableTiles)
                    FLASHLIGHT_TILE_SPEC,
                    .isEqualTo(
            ).mapTo(mutableSetOf(), TileSpec::create))
                        setOf(AIRPLANE_MODE_TILE_SPEC, WORK_MODE_TILE_SPEC, FLASHLIGHT_TILE_SPEC)
                            .mapTo(mutableSetOf(), TileSpec::create)
                    )
            }
            }
        }
        }


    @Test
    @Test
    fun tileCannotBeCreated_isUnavailable() = with(kosmos) {
    fun tileCannotBeCreated_isUnavailable() =
        with(kosmos) {
            testScope.runTest {
            testScope.runTest {
                val badSpec = TileSpec.create("unknown")
                val badSpec = TileSpec.create("unknown")
            val unavailableTiles = underTest.getUnavailableTiles(
                val unavailableTiles = underTest.getUnavailableTiles(setOf(badSpec))
                    setOf(
                        badSpec
                    )
            )
                assertThat(unavailableTiles).contains(badSpec)
                assertThat(unavailableTiles).contains(badSpec)
            }
            }
        }
        }


    @Test
    @Test
    @EnableFlags(FLAG_QS_NEW_TILES)
    @EnableFlags(FLAG_QS_NEW_TILES)
    fun flagOn_defaultsToInteractorTiles_usesFactoryForOthers() = with(kosmos) {
    fun flagOn_defaultsToInteractorTiles_usesFactoryForOthers() =
        with(kosmos) {
            testScope.runTest {
            testScope.runTest {
            val unavailableTiles = underTest.getUnavailableTiles(
                val unavailableTiles =
                    underTest.getUnavailableTiles(
                        setOf(
                        setOf(
                                AIRPLANE_MODE_TILE_SPEC,
                                AIRPLANE_MODE_TILE_SPEC,
                                WORK_MODE_TILE_SPEC,
                                WORK_MODE_TILE_SPEC,
                                HOTSPOT_TILE_SPEC,
                                HOTSPOT_TILE_SPEC,
                                INTERNET_TILE_SPEC,
                                INTERNET_TILE_SPEC,
                                FLASHLIGHT_TILE_SPEC,
                                FLASHLIGHT_TILE_SPEC,
                    ).map(TileSpec::create)
                            )
                            )
            assertThat(unavailableTiles).isEqualTo(setOf(
                            .map(TileSpec::create)
                    HOTSPOT_TILE_SPEC,
                    )
                    FLASHLIGHT_TILE_SPEC,
                assertThat(unavailableTiles)
            ).mapTo(mutableSetOf(), TileSpec::create))
                    .isEqualTo(
                        setOf(HOTSPOT_TILE_SPEC, FLASHLIGHT_TILE_SPEC)
                            .mapTo(mutableSetOf(), TileSpec::create)
                    )
            }
            }
        }
        }


    @Test
    @Test
    @EnableFlags(FLAG_QS_NEW_TILES)
    @EnableFlags(FLAG_QS_NEW_TILES)
    fun flagOn_defaultsToInteractorTiles_usesFactoryForOthers_userChange() = with(kosmos) {
    fun flagOn_defaultsToInteractorTiles_usesFactoryForOthers_userChange() =
        with(kosmos) {
            testScope.runTest {
            testScope.runTest {
                fakeUserRepository.asMainUser()
                fakeUserRepository.asMainUser()
            val unavailableTiles = underTest.getUnavailableTiles(
                val unavailableTiles =
                    underTest.getUnavailableTiles(
                        setOf(
                        setOf(
                                AIRPLANE_MODE_TILE_SPEC,
                                AIRPLANE_MODE_TILE_SPEC,
                                WORK_MODE_TILE_SPEC,
                                WORK_MODE_TILE_SPEC,
                                HOTSPOT_TILE_SPEC,
                                HOTSPOT_TILE_SPEC,
                                INTERNET_TILE_SPEC,
                                INTERNET_TILE_SPEC,
                                FLASHLIGHT_TILE_SPEC,
                                FLASHLIGHT_TILE_SPEC,
                    ).map(TileSpec::create)
                            )
                            )
            assertThat(unavailableTiles).isEqualTo(setOf(
                            .map(TileSpec::create)
                    WORK_MODE_TILE_SPEC,
                    )
                    HOTSPOT_TILE_SPEC,
                assertThat(unavailableTiles)
                    FLASHLIGHT_TILE_SPEC,
                    .isEqualTo(
            ).mapTo(mutableSetOf(), TileSpec::create))
                        setOf(WORK_MODE_TILE_SPEC, HOTSPOT_TILE_SPEC, FLASHLIGHT_TILE_SPEC)
                            .mapTo(mutableSetOf(), TileSpec::create)
                    )
            }
            }
        }
        }


    @Test
    @Test
    @EnableFlags(FLAG_QS_NEW_TILES)
    @EnableFlags(FLAG_QS_NEW_TILES)
    fun flagOn_onlyNeededTilesAreCreated_andThenDestroyed() = with(kosmos) {
    fun flagOn_onlyNeededTilesAreCreated_andThenDestroyed() =
        with(kosmos) {
            testScope.runTest {
            testScope.runTest {
                underTest.getUnavailableTiles(
                underTest.getUnavailableTiles(
                    setOf(
                    setOf(
@@ -172,19 +187,22 @@ class TilesAvailabilityInteractorTest(flags: FlagsParameterization) : SysuiTestC
                            HOTSPOT_TILE_SPEC,
                            HOTSPOT_TILE_SPEC,
                            INTERNET_TILE_SPEC,
                            INTERNET_TILE_SPEC,
                            FLASHLIGHT_TILE_SPEC,
                            FLASHLIGHT_TILE_SPEC,
                    ).map(TileSpec::create)
                        )
                        .map(TileSpec::create)
                )
                )
                assertThat(createdTiles.map { it.tileSpec })
                assertThat(createdTiles.map { it.tileSpec })
                    .containsExactly(INTERNET_TILE_SPEC, FLASHLIGHT_TILE_SPEC)
                    .containsExactly(INTERNET_TILE_SPEC, FLASHLIGHT_TILE_SPEC)
            assertThat(createdTiles.all { it.destroyed }).isTrue()
                assertThat(createdTiles.all { it.isDestroyed }).isTrue()
            }
            }
        }
        }


    @Test
    @Test
    @DisableFlags(FLAG_QS_NEW_TILES)
    @DisableFlags(FLAG_QS_NEW_TILES)
    fun flagOn_TilesAreCreatedAndThenDestroyed() = with(kosmos) {
    fun flagOn_TilesAreCreatedAndThenDestroyed() =
        with(kosmos) {
            testScope.runTest {
            testScope.runTest {
            val allTiles = setOf(
                val allTiles =
                    setOf(
                        AIRPLANE_MODE_TILE_SPEC,
                        AIRPLANE_MODE_TILE_SPEC,
                        WORK_MODE_TILE_SPEC,
                        WORK_MODE_TILE_SPEC,
                        HOTSPOT_TILE_SPEC,
                        HOTSPOT_TILE_SPEC,
@@ -192,30 +210,24 @@ class TilesAvailabilityInteractorTest(flags: FlagsParameterization) : SysuiTestC
                        FLASHLIGHT_TILE_SPEC,
                        FLASHLIGHT_TILE_SPEC,
                    )
                    )
                underTest.getUnavailableTiles(allTiles.map(TileSpec::create))
                underTest.getUnavailableTiles(allTiles.map(TileSpec::create))
            assertThat(createdTiles.map { it.tileSpec })
                assertThat(createdTiles.map { it.tileSpec }).containsExactlyElementsIn(allTiles)
                    .containsExactlyElementsIn(allTiles)
                assertThat(createdTiles.all { it.isDestroyed }).isTrue()
            assertThat(createdTiles.all { it.destroyed }).isTrue()
            }
            }
        }
        }



    private fun constantFactory(creatorTiles: Set<FakeQSTile>): QSFactory {
    private fun constantFactory(creatorTiles: Set<FakeQSTile>): QSFactory {
        return FakeQSFactory { spec ->
        return FakeQSFactory { spec ->
            creatorTiles.firstOrNull { it.tileSpec == spec }?.also {
            creatorTiles.firstOrNull { it.tileSpec == spec }?.also { createdTiles.add(it) }
                createdTiles.add(it)
            }
        }
        }
    }
    }


    companion object {
    companion object {
        private fun tilesForCreator(
        private fun tilesForCreator(
            user: Int,
            user: Int,
                specAvailabilities: Map<String, Boolean>
            specAvailabilities: Map<String, Boolean>,
        ): Set<FakeQSTile> {
        ): Set<FakeQSTile> {
            return specAvailabilities.mapTo(mutableSetOf()) {
            return specAvailabilities.mapTo(mutableSetOf()) {
                FakeQSTile(user, it.value).apply {
                FakeQSTile(user, it.value).apply { tileSpec = it.key }
                    tileSpec = it.key
                }
            }
            }
        }
        }


+26 −3
Original line number Original line Diff line number Diff line
@@ -254,7 +254,7 @@ class CurrentTilesInteractorImplTest : SysuiTestCase() {
                assertThat(tiles?.size).isEqualTo(1)
                assertThat(tiles?.size).isEqualTo(1)
                assertThat(tiles!![0].spec).isEqualTo(TileSpec.create("a"))
                assertThat(tiles!![0].spec).isEqualTo(TileSpec.create("a"))


                assertThat((originalTileC as FakeQSTile).destroyed).isTrue()
                assertThat(originalTileC.isDestroyed).isTrue()
                verify(qsLogger)
                verify(qsLogger)
                    .logTileDestroyed(
                    .logTileDestroyed(
                        TileSpec.create("c"),
                        TileSpec.create("c"),
@@ -282,7 +282,7 @@ class CurrentTilesInteractorImplTest : SysuiTestCase() {
                tileSpecRepository.addTile(USER_INFO_0.id, TileSpec.create("b"))
                tileSpecRepository.addTile(USER_INFO_0.id, TileSpec.create("b"))
                runCurrent()
                runCurrent()


                assertThat(originalTileA.destroyed).isTrue()
                assertThat(originalTileA.isDestroyed).isTrue()
                verify(qsLogger)
                verify(qsLogger)
                    .logTileDestroyed(
                    .logTileDestroyed(
                        TileSpec.create("a"),
                        TileSpec.create("a"),
@@ -331,7 +331,7 @@ class CurrentTilesInteractorImplTest : SysuiTestCase() {
                switchUser(USER_INFO_1)
                switchUser(USER_INFO_1)
                runCurrent()
                runCurrent()


                assertThat((originalTileA as FakeQSTile).destroyed).isTrue()
                assertThat(originalTileA.isDestroyed).isTrue()
                verify(qsLogger)
                verify(qsLogger)
                    .logTileDestroyed(
                    .logTileDestroyed(
                        specs0[0],
                        specs0[0],
@@ -703,6 +703,29 @@ class CurrentTilesInteractorImplTest : SysuiTestCase() {
            }
            }
        }
        }


    @Test
    fun destroyedTilesNotReused() =
        with(kosmos) {
            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.setTiles(specs)

                val tileABefore = tiles!!.first { it.spec == specs[0] }.tile

                // We destroy it manually, in prod, this could happen if the tile processing action
                // is interrupted in the middle.
                tileABefore.destroy()

                underTest.addTile(newTile)

                val tileAAfter = tiles!!.first { it.spec == specs[0] }.tile
                assertThat(tileAAfter).isNotSameInstanceAs(tileABefore)
            }
        }

    private fun QSTile.State.fillIn(state: Int, label: CharSequence, secondaryLabel: CharSequence) {
    private fun QSTile.State.fillIn(state: Int, label: CharSequence, secondaryLabel: CharSequence) {
        this.state = state
        this.state = state
        this.label = label
        this.label = label
+6 −0
Original line number Original line Diff line number Diff line
@@ -508,6 +508,12 @@ public class QSTileImplTest extends SysuiTestCase {
        assertThat(mTile.mRefreshes).isEqualTo(1);
        assertThat(mTile.mRefreshes).isEqualTo(1);
    }
    }


    @Test
    public void testIsDestroyedImmediately() {
        mTile.destroy();
        assertThat(mTile.isDestroyed()).isTrue();
    }

    private void assertEvent(UiEventLogger.UiEventEnum eventType,
    private void assertEvent(UiEventLogger.UiEventEnum eventType,
            UiEventLoggerFake.FakeUiEvent fakeEvent) {
            UiEventLoggerFake.FakeUiEvent fakeEvent) {
        assertEquals(eventType.getId(), fakeEvent.eventId);
        assertEquals(eventType.getId(), fakeEvent.eventId);
+4 −1
Original line number Original line Diff line number Diff line
@@ -42,7 +42,7 @@ import java.util.function.Supplier;
@DependsOn(target = Icon.class)
@DependsOn(target = Icon.class)
@DependsOn(target = State.class)
@DependsOn(target = State.class)
public interface QSTile {
public interface QSTile {
    int VERSION = 4;
    int VERSION = 5;


    String getTileSpec();
    String getTileSpec();


@@ -78,6 +78,7 @@ public interface QSTile {
    void longClick(@Nullable Expandable expandable);
    void longClick(@Nullable Expandable expandable);


    void userSwitch(int currentUser);
    void userSwitch(int currentUser);
    int getCurrentTileUser();


    /**
    /**
     * @deprecated not needed as {@link com.android.internal.logging.UiEvent} will use
     * @deprecated not needed as {@link com.android.internal.logging.UiEvent} will use
@@ -150,6 +151,8 @@ public interface QSTile {
        return null;
        return null;
    }
    }


    boolean isDestroyed();

    @ProvidesInterface(version = Callback.VERSION)
    @ProvidesInterface(version = Callback.VERSION)
    interface Callback {
    interface Callback {
        static final int VERSION = 2;
        static final int VERSION = 2;
Loading