From f455951c665628637923f2ebabc51b56f00cde42 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Tue, 23 Jun 2020 14:02:58 -0400 Subject: [PATCH] Fix lifecycle states in QSTileImpl QSTileImpl set the lifecycle state to DESTROYED whenever it was supposed to stop listening. Due to the correct enforcement that this is a final state, it couldn't be moved out of it to RESUMED. With this CL, QSTileImpl will toggle between RESUMED and STARTED to determine it's listening state. Objects that track its lifecycle should listen for ON_RESUME and ON_PAUSE events (like CallbackController). Test: manual, toggle tiles Test: manual, observe that CallbackController#removeCallback is called Test: atest QSTileImplTest Fixes: 159680648 Change-Id: I00fae4f13c446af103587e5f064277e8d42a7b22 --- .../systemui/qs/tileimpl/QSTileImpl.java | 8 ++-- .../systemui/qs/tileimpl/QSTileImplTest.java | 43 +++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java index 87faaccf20638..795d0627c4474 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java @@ -14,8 +14,8 @@ package com.android.systemui.qs.tileimpl; -import static androidx.lifecycle.Lifecycle.State.DESTROYED; import static androidx.lifecycle.Lifecycle.State.RESUMED; +import static androidx.lifecycle.Lifecycle.State.STARTED; import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.ACTION_QS_CLICK; import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.ACTION_QS_LONG_PRESS; @@ -432,17 +432,19 @@ public abstract class QSTileImpl implements QSTile, Lifecy } private void handleSetListeningInternal(Object listener, boolean listening) { + // This should be used to go from resumed to paused. Listening for ON_RESUME and ON_PAUSE + // in this lifecycle will determine the listening window. if (listening) { if (mListeners.add(listener) && mListeners.size() == 1) { if (DEBUG) Log.d(TAG, "handleSetListening true"); - mLifecycle.markState(RESUMED); + mLifecycle.setCurrentState(RESUMED); handleSetListening(listening); refreshState(); // Ensure we get at least one refresh after listening. } } else { if (mListeners.remove(listener) && mListeners.size() == 0) { if (DEBUG) Log.d(TAG, "handleSetListening false"); - mLifecycle.markState(DESTROYED); + mLifecycle.setCurrentState(STARTED); handleSetListening(listening); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileImplTest.java index 1c0d451e064a8..438de99015a4c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileImplTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileImplTest.java @@ -14,6 +14,10 @@ package com.android.systemui.qs.tileimpl; + +import static androidx.lifecycle.Lifecycle.State.DESTROYED; +import static androidx.lifecycle.Lifecycle.State.RESUMED; + import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.ACTION_QS_CLICK; import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.ACTION_QS_LONG_PRESS; import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.ACTION_QS_SECONDARY_CLICK; @@ -23,6 +27,9 @@ import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.FIELD_ import static com.android.internal.logging.nano.MetricsProto.MetricsEvent.TYPE_ACTION; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; @@ -267,6 +274,42 @@ public class QSTileImplTest extends SysuiTestCase { verify(mQsLogger).logTileChangeListening(SPEC, false); } + @Test + public void testListeningTrue_stateAtLeastResumed() { + mTile.setListening(new Object(), true); // Listen with some object + + TestableLooper.get(this).processAllMessages(); + + assertTrue(mTile.getLifecycle().getCurrentState().isAtLeast(RESUMED)); + } + + @Test + public void testTileDoesntStartResumed() { + assertFalse(mTile.getLifecycle().getCurrentState().isAtLeast(RESUMED)); + } + + @Test + public void testListeningFalse_stateAtMostCreated() { + Object o = new Object(); + mTile.setListening(o, true); + + mTile.setListening(o, false); + + TestableLooper.get(this).processAllMessages(); + assertFalse(mTile.getLifecycle().getCurrentState().isAtLeast(RESUMED)); + } + + @Test + public void testListeningFalse_stateNotDestroyed() { + Object o = new Object(); + mTile.setListening(o, true); + + mTile.setListening(o, false); + + TestableLooper.get(this).processAllMessages(); + assertNotEquals(DESTROYED, mTile.getLifecycle().getCurrentState()); + } + private void assertEvent(UiEventLogger.UiEventEnum eventType, UiEventLoggerFake.FakeUiEvent fakeEvent) { assertEquals(eventType.getId(), fakeEvent.eventId);