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

Commit 8cf0a2a2 authored by Fabian Kozynski's avatar Fabian Kozynski Committed by Colin Cross
Browse files

setCurrentState(DESTROYED) called from main thread

Also address timing issues with bouncing calls between 2 handlers. In
particular, make sure that after a tile is DESTROYED, the state doesn't
change anymore due to leftover refreshState calls.

This sometimes would be caused as the result of an earlier tile added as
a callback on something that replies on add.

Make sure that tests that instantiate a QSTileImpl use the main looper
for the passed handler.

Test: atest TileServiceTest BooleanTileServiceTest
Test: manual, open QSCustomizer that destroys tiles
Test: atest SystemUITests
Change-Id: I77c61f743510cdcca54fa37cd26062a3c96bd649
Merged-In: I77c61f743510cdcca54fa37cd26062a3c96bd649
(cherry picked from commit 3744d134)
parent 6af74807
Loading
Loading
Loading
Loading
+14 −2
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@

package com.android.systemui.qs.tileimpl;

import static androidx.lifecycle.Lifecycle.State.CREATED;
import static androidx.lifecycle.Lifecycle.State.DESTROYED;
import static androidx.lifecycle.Lifecycle.State.RESUMED;
import static androidx.lifecycle.Lifecycle.State.STARTED;
@@ -173,6 +174,7 @@ public abstract class QSTileImpl<TState extends State> implements QSTile, Lifecy

        mState = newTileState();
        mTmpState = newTileState();
        mUiHandler.post(() -> mLifecycle.setCurrentState(CREATED));
    }

    protected final void resetStates() {
@@ -453,6 +455,9 @@ public abstract class QSTileImpl<TState extends State> implements QSTile, Lifecy
                if (DEBUG) Log.d(TAG, "handleSetListening true");
                handleSetListening(listening);
                mUiHandler.post(() -> {
                    // This tile has been destroyed, the state should not change anymore and we
                    // should not refresh it anymore.
                    if (mLifecycle.getCurrentState().equals(DESTROYED)) return;
                    mLifecycle.setCurrentState(RESUMED);
                    refreshState(); // Ensure we get at least one refresh after listening.
                });
@@ -461,7 +466,11 @@ public abstract class QSTileImpl<TState extends State> implements QSTile, Lifecy
            if (mListeners.remove(listener) && mListeners.size() == 0) {
                if (DEBUG) Log.d(TAG, "handleSetListening false");
                handleSetListening(listening);
                mUiHandler.post(() -> mLifecycle.setCurrentState(STARTED));
                mUiHandler.post(() -> {
                    // This tile has been destroyed, the state should not change anymore.
                    if (mLifecycle.getCurrentState().equals(DESTROYED)) return;
                    mLifecycle.setCurrentState(STARTED);
                });
            }
        }
        updateIsFullQs();
@@ -488,11 +497,14 @@ public abstract class QSTileImpl<TState extends State> implements QSTile, Lifecy
        mQSLogger.logTileDestroyed(mTileSpec, "Handle destroy");
        if (mListeners.size() != 0) {
            handleSetListening(false);
            mListeners.clear();
        }
        mCallbacks.clear();
        mHandler.removeCallbacksAndMessages(null);
        // This will force it to be removed from all controllers that may have it registered.
        mUiHandler.post(() -> {
            mLifecycle.setCurrentState(DESTROYED);
        });
    }

    protected void checkIfRestrictionEnforcedByAdminOnly(State state, String userRestriction) {
+1 −1
Original line number Diff line number Diff line
@@ -78,7 +78,7 @@ import javax.inject.Provider;

@RunWith(AndroidTestingRunner.class)
@SmallTest
@RunWithLooper
@RunWithLooper(setAsMainLooper = true)
public class QSTileHostTest extends SysuiTestCase {

    private static String MOCK_STATE_STRING = "MockState";
+1 −1
Original line number Diff line number Diff line
@@ -53,7 +53,7 @@ import org.mockito.MockitoAnnotations

@SmallTest
@RunWith(AndroidTestingRunner::class)
@TestableLooper.RunWithLooper
@TestableLooper.RunWithLooper(setAsMainLooper = true)
class CustomTileTest : SysuiTestCase() {

    companion object {
+21 −0
Original line number Diff line number Diff line
@@ -244,6 +244,8 @@ public class QSTileImplTest extends SysuiTestCase {
        assertNotEquals(DESTROYED, mTile.getLifecycle().getCurrentState());
        mTile.handleDestroy();

        mTestableLooper.processAllMessages();

        assertEquals(DESTROYED, mTile.getLifecycle().getCurrentState());
    }

@@ -298,6 +300,25 @@ public class QSTileImplTest extends SysuiTestCase {
        assertNotEquals(DESTROYED, mTile.getLifecycle().getCurrentState());
    }

    @Test
    public void testRefreshStateAfterDestroyedDoesNotCrash() {
        mTile.destroy();
        mTile.refreshState();

        mTestableLooper.processAllMessages();
    }

    @Test
    public void testSetListeningAfterDestroyedDoesNotCrash() {
        Object o = new Object();
        mTile.destroy();

        mTile.setListening(o, true);
        mTile.setListening(o, false);

        mTestableLooper.processAllMessages();
    }

    private void assertEvent(UiEventLogger.UiEventEnum eventType,
            UiEventLoggerFake.FakeUiEvent fakeEvent) {
        assertEquals(eventType.getId(), fakeEvent.eventId);
+1 −1
Original line number Diff line number Diff line
@@ -38,7 +38,7 @@ import org.mockito.Mockito.`when`
import org.mockito.MockitoAnnotations

@RunWith(AndroidTestingRunner::class)
@RunWithLooper
@RunWithLooper(setAsMainLooper = true)
@SmallTest
class BatterySaverTileTest : SysuiTestCase() {

Loading