From 59e7a219160b9755bd6b06c339af0b48532499e1 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Mon, 14 Sep 2020 16:08:08 -0400 Subject: [PATCH] Wait for state refreshed in TileQueryHelper When adding the stock tiles for TileAdapter, TileQueryHelper will set them to listening for a brief period of time to refresh the state and show the current state. It used to work because all the operations were done in the background thread. After switching the lifecycle logic to the main thread, there was a race condition preventing the new state (between RESUMED and STARTED) to be collected before it was copied. This CL adds a class TileCollector that will wait until all tiles have properly refreshed at least once before collecting their state. Test: manual Test: atest TileQueryHelperTest Fixes: 168444501 Change-Id: I5f356ae4648e837cbe1bcdbaec9ba6d5b7c09891 --- .../android/systemui/plugins/qs/QSTile.java | 4 + .../qs/customize/TileQueryHelper.java | 87 +++++++++-- .../systemui/qs/tileimpl/QSTileImpl.java | 23 ++- .../qs/customize/TileQueryHelperTest.java | 141 +++++++++++++++++- 4 files changed, 236 insertions(+), 19 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 e246917842b0f..26cead2063104 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 @@ -86,6 +86,10 @@ public interface QSTile { */ InstanceId getInstanceId(); + default boolean isTileReady() { + return false; + } + @ProvidesInterface(version = Callback.VERSION) public interface Callback { public static final int VERSION = 1; diff --git a/packages/SystemUI/src/com/android/systemui/qs/customize/TileQueryHelper.java b/packages/SystemUI/src/com/android/systemui/qs/customize/TileQueryHelper.java index db77e08c204b3..73c6504b99833 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/customize/TileQueryHelper.java +++ b/packages/SystemUI/src/com/android/systemui/qs/customize/TileQueryHelper.java @@ -80,8 +80,6 @@ public class TileQueryHelper { mFinished = false; // Enqueue jobs to fetch every system tile and then ever package tile. addCurrentAndStockTiles(host); - - addPackageTiles(host); } public boolean isFinished() { @@ -122,23 +120,86 @@ public class TileQueryHelper { tile.destroy(); continue; } - tile.setListening(this, true); - tile.refreshState(); - tile.setListening(this, false); tile.setTileSpec(spec); tilesToAdd.add(tile); } - mBgExecutor.execute(() -> { - for (QSTile tile : tilesToAdd) { - final QSTile.State state = tile.getState().copy(); - // Ignore the current state and get the generic label instead. - state.label = tile.getTileLabel(); - tile.destroy(); - addTile(tile.getTileSpec(), null, state, true); + new TileCollector(tilesToAdd, host).startListening(); + } + + private static class TilePair { + QSTile mTile; + boolean mReady = false; + } + + private class TileCollector implements QSTile.Callback { + + private final List mQSTileList = new ArrayList<>(); + private final QSTileHost mQSTileHost; + + TileCollector(List tilesToAdd, QSTileHost host) { + for (QSTile tile: tilesToAdd) { + TilePair pair = new TilePair(); + pair.mTile = tile; + mQSTileList.add(pair); } + mQSTileHost = host; + if (tilesToAdd.isEmpty()) { + mBgExecutor.execute(this::finished); + } + } + + private void finished() { notifyTilesChanged(false); - }); + addPackageTiles(mQSTileHost); + } + + private void startListening() { + for (TilePair pair: mQSTileList) { + pair.mTile.addCallback(this); + pair.mTile.setListening(this, true); + // Make sure that at least one refresh state happens + pair.mTile.refreshState(); + } + } + + // This is called in the Bg thread + @Override + public void onStateChanged(State s) { + boolean allReady = true; + for (TilePair pair: mQSTileList) { + if (!pair.mReady && pair.mTile.isTileReady()) { + pair.mTile.removeCallback(this); + pair.mTile.setListening(this, false); + pair.mReady = true; + } else if (!pair.mReady) { + allReady = false; + } + } + if (allReady) { + for (TilePair pair : mQSTileList) { + QSTile tile = pair.mTile; + final QSTile.State state = tile.getState().copy(); + // Ignore the current state and get the generic label instead. + state.label = tile.getTileLabel(); + tile.destroy(); + addTile(tile.getTileSpec(), null, state, true); + } + finished(); + } + } + + @Override + public void onShowDetail(boolean show) {} + + @Override + public void onToggleStateChanged(boolean state) {} + + @Override + public void onScanStateChanged(boolean state) {} + + @Override + public void onAnnouncementRequested(CharSequence announcement) {} } private void addPackageTiles(final QSTileHost host) { 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 255513a31c752..dfd7e2c8fdb73 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileImpl.java @@ -90,6 +90,10 @@ public abstract class QSTileImpl implements QSTile, Lifecy private static final long DEFAULT_STALE_TIMEOUT = 10 * DateUtils.MINUTE_IN_MILLIS; protected static final Object ARG_SHOW_TRANSIENT_ENABLING = new Object(); + private static final int READY_STATE_NOT_READY = 0; + private static final int READY_STATE_READYING = 1; + private static final int READY_STATE_READY = 2; + protected final QSHost mHost; protected final Context mContext; // @NonFinalForTesting @@ -101,6 +105,7 @@ public abstract class QSTileImpl implements QSTile, Lifecy protected final ActivityStarter mActivityStarter; private final UiEventLogger mUiEventLogger; private final QSLogger mQSLogger; + private volatile int mReadyState; private final ArrayList mCallbacks = new ArrayList<>(); private final Object mStaleListener = new Object(); @@ -386,7 +391,11 @@ public abstract class QSTileImpl implements QSTile, Lifecy protected void handleRefreshState(Object arg) { handleUpdateState(mTmpState, arg); - final boolean changed = mTmpState.copyTo(mState); + boolean changed = mTmpState.copyTo(mState); + if (mReadyState == READY_STATE_READYING) { + mReadyState = READY_STATE_READY; + changed = true; + } if (changed) { mQSLogger.logTileUpdated(mTileSpec, mState); handleStateChanged(); @@ -459,6 +468,9 @@ public abstract class QSTileImpl implements QSTile, Lifecy // should not refresh it anymore. if (mLifecycle.getCurrentState().equals(DESTROYED)) return; mLifecycle.setCurrentState(RESUMED); + if (mReadyState == READY_STATE_NOT_READY) { + mReadyState = READY_STATE_READYING; + } refreshState(); // Ensure we get at least one refresh after listening. }); } @@ -531,6 +543,15 @@ public abstract class QSTileImpl implements QSTile, Lifecy */ public abstract CharSequence getTileLabel(); + /** + * @return {@code true} if the tile has refreshed state at least once after having set its + * lifecycle to {@link Lifecycle.State#RESUMED}. + */ + @Override + public boolean isTileReady() { + return mReadyState == READY_STATE_READY; + } + public static int getColorForState(Context context, int state) { switch (state) { case Tile.STATE_UNAVAILABLE: 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 53ef866608673..2ef7c65acb0b3 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 @@ -34,6 +34,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.Manifest; +import android.content.Context; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; @@ -47,8 +48,11 @@ import android.util.ArraySet; import androidx.test.filters.SmallTest; +import com.android.internal.logging.InstanceId; import com.android.systemui.R; import com.android.systemui.SysuiTestCase; +import com.android.systemui.plugins.qs.DetailAdapter; +import com.android.systemui.plugins.qs.QSIconView; import com.android.systemui.plugins.qs.QSTile; import com.android.systemui.qs.QSTileHost; import com.android.systemui.util.concurrency.FakeExecutor; @@ -68,6 +72,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Set; +import java.util.concurrent.Executor; @SmallTest @RunWith(AndroidTestingRunner.class) @@ -116,11 +121,9 @@ public class TileQueryHelperTest extends SysuiTestCase { doAnswer(invocation -> { String spec = (String) invocation.getArguments()[0]; if (FACTORY_TILES.contains(spec)) { - QSTile m = mock(QSTile.class); - when(m.isAvailable()).thenReturn(true); - when(m.getTileSpec()).thenReturn(spec); - when(m.getState()).thenReturn(mState); - return m; + FakeQSTile tile = new FakeQSTile(mBgExecutor, mMainExecutor); + tile.setState(mState); + return tile; } else { return null; } @@ -292,4 +295,132 @@ public class TileQueryHelperTest extends SysuiTestCase { verifier.verify(t).setTileSpec("hotspot"); verifier.verify(t).destroy(); } + + private static class FakeQSTile implements QSTile { + + private String mSpec = ""; + private List mCallbacks = new ArrayList<>(); + private boolean mRefreshed; + private boolean mListening; + private State mState = new State(); + private final Executor mBgExecutor; + private final Executor mMainExecutor; + + FakeQSTile(Executor bgExecutor, Executor mainExecutor) { + mBgExecutor = bgExecutor; + mMainExecutor = mainExecutor; + } + + @Override + public String getTileSpec() { + return mSpec; + } + + @Override + public boolean isAvailable() { + return true; + } + + @Override + public void setTileSpec(String tileSpec) { + mSpec = tileSpec; + } + + public void setState(State state) { + mState = state; + notifyChangedState(mState); + } + + @Override + public void refreshState() { + mBgExecutor.execute(() -> { + mRefreshed = true; + notifyChangedState(mState); + }); + } + + private void notifyChangedState(State state) { + List callbacks = new ArrayList<>(mCallbacks); + callbacks.forEach(callback -> callback.onStateChanged(state)); + } + + @Override + public void addCallback(Callback callback) { + mCallbacks.add(callback); + } + + @Override + public void removeCallback(Callback callback) { + mCallbacks.remove(callback); + } + + @Override + public void removeCallbacks() { + mCallbacks.clear(); + } + + @Override + public void setListening(Object client, boolean listening) { + if (listening) { + mMainExecutor.execute(() -> { + mListening = true; + refreshState(); + }); + } + } + + @Override + public CharSequence getTileLabel() { + return mSpec; + } + + @Override + public State getState() { + return mState; + } + + @Override + public boolean isTileReady() { + return mListening && mRefreshed; + } + + @Override + public QSIconView createTileView(Context context) { + return null; + } + + @Override + public void click() {} + + @Override + public void secondaryClick() {} + + @Override + public void longClick() {} + + @Override + public void userSwitch(int currentUser) {} + + @Override + public int getMetricsCategory() { + return 0; + } + + @Override + public InstanceId getInstanceId() { + return null; + } + + @Override + public void setDetailListening(boolean show) {} + + @Override + public void destroy() {} + + + @Override + public DetailAdapter getDetailAdapter() { + return null; + } + } }