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

Commit 3744d134 authored by Fabian Kozynski's avatar Fabian Kozynski
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
parent 0a3b7c6a
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