From 0ca203c23cfcdc8c4915d3ee68b6c60954a4d212 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Thu, 24 Mar 2022 10:30:27 -0400 Subject: [PATCH] Fix refresh tile when tile is already listening Calling refreshTile externally on a tile that is already listening can have unexpected side effects, for example if the tile is tracking a user interaction (e.g. turning on bt). In particular, QSPanelController should not call it for tiles that are already listening. This is the case for tiles that are also in QQS. Test: manual, toggle bt ON in QQS and expand quickly. Test: atest com.android.systemui.qs Fixes: 225404999 Change-Id: Id0559ad95ecf6a28475abbdd8b989c745badd0b8 --- .../android/systemui/plugins/qs/QSTile.java | 8 ++++++- .../systemui/qs/QSPanelControllerBase.java | 7 +++++- .../systemui/qs/tileimpl/QSTileImpl.java | 7 +++++- .../qs/QSPanelControllerBaseTest.java | 17 ++++++++++++++ .../systemui/qs/QSPanelControllerTest.kt | 22 ++++++++++++++----- .../qs/customize/TileQueryHelperTest.java | 5 +++++ .../systemui/qs/tileimpl/QSTileImplTest.java | 18 +++++++++++++++ 7 files changed, 76 insertions(+), 8 deletions(-) diff --git a/packages/SystemUI/plugin/src/com/android/systemui/plugins/qs/QSTile.java b/packages/SystemUI/plugin/src/com/android/systemui/plugins/qs/QSTile.java index 669d6a3835ac5..2b169997168b7 100644 --- a/packages/SystemUI/plugin/src/com/android/systemui/plugins/qs/QSTile.java +++ b/packages/SystemUI/plugin/src/com/android/systemui/plugins/qs/QSTile.java @@ -39,7 +39,7 @@ import java.util.function.Supplier; @DependsOn(target = Icon.class) @DependsOn(target = State.class) public interface QSTile { - int VERSION = 3; + int VERSION = 4; String getTileSpec(); @@ -114,6 +114,12 @@ public interface QSTile { return false; } + /** + * Return whether the tile is set to its listening state and therefore receiving updates and + * refreshes from controllers + */ + boolean isListening(); + @ProvidesInterface(version = Callback.VERSION) interface Callback { static final int VERSION = 2; diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java b/packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java index 58007c0d370a2..74d1a3d43ab32 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java @@ -217,7 +217,12 @@ public abstract class QSPanelControllerBase extends ViewContr /** */ public void refreshAllTiles() { for (QSPanelControllerBase.TileRecord r : mRecords) { - r.tile.refreshState(); + if (!r.tile.isListening()) { + // Only refresh tiles that were not already in the listening state. Tiles that are + // already listening is as if they are already expanded (for example, tiles that + // are both in QQS and QS). + r.tile.refreshState(); + } } } 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 999818ff943fa..8755bc90c0a8d 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java @@ -331,6 +331,11 @@ public abstract class QSTileImpl implements QSTile, Lifecy refreshState(null); } + @Override + public final boolean isListening() { + return getLifecycle().getCurrentState().isAtLeast(RESUMED); + } + protected final void refreshState(@Nullable Object arg) { mHandler.obtainMessage(H.REFRESH_STATE, arg).sendToTarget(); } @@ -416,7 +421,7 @@ public abstract class QSTileImpl implements QSTile, Lifecy @Nullable public abstract Intent getLongClickIntent(); - protected void handleRefreshState(@Nullable Object arg) { + protected final void handleRefreshState(@Nullable Object arg) { handleUpdateState(mTmpState, arg); boolean changed = mTmpState.copyTo(mState); if (mReadyState == READY_STATE_READYING) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerBaseTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerBaseTest.java index 8ccf5596b0ec0..3f45ff321013d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerBaseTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerBaseTest.java @@ -46,6 +46,7 @@ import com.android.systemui.R; import com.android.systemui.SysuiTestCase; import com.android.systemui.dump.DumpManager; import com.android.systemui.media.MediaHost; +import com.android.systemui.plugins.qs.QSTile; import com.android.systemui.plugins.qs.QSTileView; import com.android.systemui.qs.customize.QSCustomizerController; import com.android.systemui.qs.logging.QSLogger; @@ -62,6 +63,7 @@ import java.io.FileDescriptor; import java.io.PrintWriter; import java.io.StringWriter; import java.util.Collections; +import java.util.List; @RunWith(AndroidTestingRunner.class) @RunWithLooper @@ -89,6 +91,8 @@ public class QSPanelControllerBaseTest extends SysuiTestCase { @Mock QSTileImpl mQSTile; @Mock + QSTile mOtherTile; + @Mock QSTileView mQSTileView; @Mock PagedTileLayout mPagedTileLayout; @@ -280,4 +284,17 @@ public class QSPanelControllerBaseTest extends SysuiTestCase { assertThat(mController.shouldUseHorizontalLayout()).isFalse(); verify(mHorizontalLayoutListener, times(2)).run(); } + + @Test + public void testRefreshAllTilesDoesntRefreshListeningTiles() { + when(mQSTileHost.getTiles()).thenReturn(List.of(mQSTile, mOtherTile)); + mController.setTiles(); + + when(mQSTile.isListening()).thenReturn(false); + when(mOtherTile.isListening()).thenReturn(true); + + mController.refreshAllTiles(); + verify(mQSTile).refreshState(); + verify(mOtherTile, never()).refreshState(); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerTest.kt index 1b48a16740b99..e9488e9ad98c2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerTest.kt @@ -4,13 +4,13 @@ import android.test.suitebuilder.annotation.SmallTest import android.testing.AndroidTestingRunner import com.android.internal.logging.MetricsLogger import com.android.internal.logging.UiEventLogger -import com.android.systemui.R import com.android.systemui.SysuiTestCase import com.android.systemui.dump.DumpManager import com.android.systemui.flags.FeatureFlags import com.android.systemui.media.MediaHost import com.android.systemui.media.MediaHostState import com.android.systemui.plugins.FalsingManager +import com.android.systemui.plugins.qs.QSTile import com.android.systemui.qs.customize.QSCustomizerController import com.android.systemui.qs.logging.QSLogger import com.android.systemui.settings.brightness.BrightnessController @@ -21,11 +21,12 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock +import org.mockito.Mockito import org.mockito.Mockito.any import org.mockito.Mockito.reset import org.mockito.Mockito.verify -import org.mockito.Mockito.`when` as whenever import org.mockito.MockitoAnnotations +import org.mockito.Mockito.`when` as whenever @SmallTest @RunWith(AndroidTestingRunner::class) @@ -49,6 +50,8 @@ class QSPanelControllerTest : SysuiTestCase() { @Mock private lateinit var falsingManager: FalsingManager @Mock private lateinit var featureFlags: FeatureFlags @Mock private lateinit var mediaHost: MediaHost + @Mock private lateinit var tile: QSTile + @Mock private lateinit var otherTile: QSTile private lateinit var controller: QSPanelController @@ -93,8 +96,17 @@ class QSPanelControllerTest : SysuiTestCase() { verify(mediaHost).expansion = MediaHostState.EXPANDED } - private fun setSplitShadeEnabled(enabled: Boolean) { - mContext.orCreateTestableResources - .addOverride(R.bool.config_use_split_notification_shade, enabled) + @Test + fun testSetListeningDoesntRefreshListeningTiles() { + whenever(qsTileHost.getTiles()).thenReturn(listOf(tile, otherTile)) + controller.setTiles() + whenever(tile.isListening()).thenReturn(false) + whenever(otherTile.isListening()).thenReturn(true) + whenever(qsPanel.isListening).thenReturn(true) + + controller.setListening(true, true) + + verify(tile).refreshState() + verify(otherTile, Mockito.never()).refreshState() } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileQueryHelperTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileQueryHelperTest.java index 30b464b2557c3..040af70f20777 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileQueryHelperTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileQueryHelperTest.java @@ -372,6 +372,11 @@ public class TileQueryHelperTest extends SysuiTestCase { } } + @Override + public boolean isListening() { + return mListening; + } + @Override public CharSequence getTileLabel() { return mSpec; 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 c5bc68d6bd7bd..c31a971cc7756 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 @@ -361,6 +361,24 @@ public class QSTileImplTest extends SysuiTestCase { assertEquals(Settings.ACTION_SHOW_ADMIN_SUPPORT_DETAILS, captor.getValue().getAction()); } + @Test + public void testIsListening() { + Object o = new Object(); + + mTile.setListening(o, true); + mTestableLooper.processAllMessages(); + assertTrue(mTile.isListening()); + + mTile.setListening(o, false); + mTestableLooper.processAllMessages(); + assertFalse(mTile.isListening()); + + mTile.setListening(o, true); + mTile.destroy(); + mTestableLooper.processAllMessages(); + assertFalse(mTile.isListening()); + } + private void assertEvent(UiEventLogger.UiEventEnum eventType, UiEventLoggerFake.FakeUiEvent fakeEvent) { assertEquals(eventType.getId(), fakeEvent.eventId);