Merge "Fix race condition in QSTileHost" into tm-dev
diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java b/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java
index fbdabc7..fcd9e10 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java
@@ -19,8 +19,6 @@
import android.content.Intent;
import android.content.res.Resources;
import android.os.Build;
-import android.os.Handler;
-import android.os.Looper;
import android.os.UserHandle;
import android.os.UserManager;
import android.provider.Settings.Secure;
@@ -28,6 +26,7 @@
import android.util.ArraySet;
import android.util.Log;
+import androidx.annotation.MainThread;
import androidx.annotation.Nullable;
import com.android.internal.logging.InstanceId;
@@ -35,9 +34,7 @@
import com.android.internal.logging.UiEventLogger;
import com.android.systemui.Dumpable;
import com.android.systemui.R;
-import com.android.systemui.broadcast.BroadcastDispatcher;
import com.android.systemui.dagger.SysUISingleton;
-import com.android.systemui.dagger.qualifiers.Background;
import com.android.systemui.dagger.qualifiers.Main;
import com.android.systemui.dump.DumpManager;
import com.android.systemui.plugins.PluginListener;
@@ -68,12 +65,20 @@
import java.util.List;
import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.Executor;
import java.util.function.Predicate;
import javax.inject.Inject;
import javax.inject.Provider;
-/** Platform implementation of the quick settings tile host **/
+/** Platform implementation of the quick settings tile host
+ *
+ * This class keeps track of the set of current tiles and is the in memory source of truth
+ * (ground truth is kept in {@link Secure#QS_TILES}). When the ground truth changes,
+ * {@link #onTuningChanged} will be called and the tiles will be re-created as needed.
+ *
+ * This class also provides the interface for adding/removing/changing tiles.
+ */
@SysUISingleton
public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, Dumpable {
private static final String TAG = "QSTileHost";
@@ -89,11 +94,11 @@
private final TunerService mTunerService;
private final PluginManager mPluginManager;
private final DumpManager mDumpManager;
- private final BroadcastDispatcher mBroadcastDispatcher;
private final QSLogger mQSLogger;
private final UiEventLogger mUiEventLogger;
private final InstanceIdSequence mInstanceIdSequence;
private final CustomTileStatePersister mCustomTileStatePersister;
+ private final Executor mMainExecutor;
private final List<Callback> mCallbacks = new ArrayList<>();
@Nullable
@@ -113,13 +118,11 @@
public QSTileHost(Context context,
StatusBarIconController iconController,
QSFactory defaultFactory,
- @Main Handler mainHandler,
- @Background Looper bgLooper,
+ @Main Executor mainExecutor,
PluginManager pluginManager,
TunerService tunerService,
Provider<AutoTileManager> autoTiles,
DumpManager dumpManager,
- BroadcastDispatcher broadcastDispatcher,
Optional<CentralSurfaces> centralSurfacesOptional,
QSLogger qsLogger,
UiEventLogger uiEventLogger,
@@ -137,7 +140,7 @@
mDumpManager = dumpManager;
mQSLogger = qsLogger;
mUiEventLogger = uiEventLogger;
- mBroadcastDispatcher = broadcastDispatcher;
+ mMainExecutor = mainExecutor;
mTileServiceRequestController = tileServiceRequestControllerBuilder.create(this);
mTileLifeCycleManagerFactory = tileLifecycleManagerFactory;
@@ -151,7 +154,7 @@
mSecureSettings = secureSettings;
mCustomTileStatePersister = customTileStatePersister;
- mainHandler.post(() -> {
+ mainExecutor.execute(() -> {
// This is technically a hack to avoid circular dependency of
// QSTileHost -> XXXTile -> QSTileHost. Posting ensures creation
// finishes before creating any tiles.
@@ -258,6 +261,33 @@
return mTileSpecs.indexOf(spec);
}
+ /**
+ * Whenever the Secure Setting keeping track of the current tiles changes (or upon start) this
+ * will be called with the new value of the setting.
+ *
+ * This method will do the following:
+ * <ol>
+ * <li>Destroy any existing tile that's not one of the current tiles (in the setting)</li>
+ * <li>Create new tiles for those that don't already exist. If this tiles end up being
+ * not available, they'll also be destroyed.</li>
+ * <li>Save the resolved list of tiles (current tiles that are available) into the setting.
+ * This means that after this call ends, the tiles in the Setting, {@link #mTileSpecs},
+ * and visible tiles ({@link #mTiles}) must match.
+ * </li>
+ * </ol>
+ *
+ * Additionally, if the user has changed, it'll do the following:
+ * <ul>
+ * <li>Change the user for SystemUI tiles: {@link QSTile#userSwitch}.</li>
+ * <li>Destroy any {@link CustomTile} and recreate it for the new user.</li>
+ * </ul>
+ *
+ * This happens in main thread as {@link com.android.systemui.tuner.TunerServiceImpl} dispatches
+ * in main thread.
+ *
+ * @see QSTile#isAvailable
+ */
+ @MainThread
@Override
public void onTuningChanged(String key, String newValue) {
if (!TILES_SETTING.equals(key)) {
@@ -330,34 +360,44 @@
mCurrentUser = currentUser;
List<String> currentSpecs = new ArrayList<>(mTileSpecs);
mTileSpecs.clear();
- mTileSpecs.addAll(tileSpecs);
+ mTileSpecs.addAll(newTiles.keySet()); // Only add the valid (available) tiles.
mTiles.clear();
mTiles.putAll(newTiles);
if (newTiles.isEmpty() && !tileSpecs.isEmpty()) {
// If we didn't manage to create any tiles, set it to empty (default)
Log.d(TAG, "No valid tiles on tuning changed. Setting to default.");
- changeTiles(currentSpecs, loadTileSpecs(mContext, ""));
+ changeTilesByUser(currentSpecs, loadTileSpecs(mContext, ""));
} else {
+ String resolvedTiles = TextUtils.join(",", mTileSpecs);
+ if (!resolvedTiles.equals(newValue)) {
+ // If the resolved tiles (those we actually ended up with) are different than
+ // the ones that are in the setting, update the Setting.
+ saveTilesToSettings(mTileSpecs);
+ }
for (int i = 0; i < mCallbacks.size(); i++) {
mCallbacks.get(i).onTilesChanged();
}
}
}
+ /**
+ * Only use with [CustomTile] if the tile doesn't exist anymore (and therefore doesn't need
+ * its lifecycle terminated).
+ */
@Override
public void removeTile(String spec) {
- changeTileSpecs(tileSpecs-> tileSpecs.remove(spec));
+ mMainExecutor.execute(() -> changeTileSpecs(tileSpecs-> tileSpecs.remove(spec)));
}
/**
* Remove many tiles at once.
*
- * It will only save to settings once (as opposed to {@link QSTileHost#removeTile} called
+ * It will only save to settings once (as opposed to {@link QSTileHost#removeTileByUser} called
* multiple times).
*/
@Override
public void removeTiles(Collection<String> specs) {
- changeTileSpecs(tileSpecs -> tileSpecs.removeAll(specs));
+ mMainExecutor.execute(() -> changeTileSpecs(tileSpecs -> tileSpecs.removeAll(specs)));
}
@Override
@@ -381,27 +421,30 @@
* @param requestPosition -1 for end, 0 for beginning, or X for insertion at position X
*/
public void addTile(String spec, int requestPosition) {
- if (spec.equals("work")) Log.wtfStack(TAG, "Adding work tile");
- changeTileSpecs(tileSpecs -> {
- if (tileSpecs.contains(spec)) return false;
+ mMainExecutor.execute(() ->
+ changeTileSpecs(tileSpecs -> {
+ if (tileSpecs.contains(spec)) return false;
- int size = tileSpecs.size();
- if (requestPosition == POSITION_AT_END || requestPosition >= size) {
- tileSpecs.add(spec);
- } else {
- tileSpecs.add(requestPosition, spec);
- }
- return true;
- });
+ int size = tileSpecs.size();
+ if (requestPosition == POSITION_AT_END || requestPosition >= size) {
+ tileSpecs.add(spec);
+ } else {
+ tileSpecs.add(requestPosition, spec);
+ }
+ return true;
+ })
+ );
}
- void saveTilesToSettings(List<String> tileSpecs) {
- if (tileSpecs.contains("work")) Log.wtfStack(TAG, "Saving work tile");
+
+ @MainThread
+ private void saveTilesToSettings(List<String> tileSpecs) {
mSecureSettings.putStringForUser(TILES_SETTING, TextUtils.join(",", tileSpecs),
null /* tag */, false /* default */, mCurrentUser,
true /* overrideable by restore */);
}
+ @MainThread
private void changeTileSpecs(Predicate<List<String>> changeFunction) {
final String setting = mSecureSettings.getStringForUser(TILES_SETTING, mCurrentUser);
final List<String> tileSpecs = loadTileSpecs(mContext, setting);
@@ -421,29 +464,32 @@
*/
public void addTile(ComponentName tile, boolean end) {
String spec = CustomTile.toSpec(tile);
- if (!mTileSpecs.contains(spec)) {
- List<String> newSpecs = new ArrayList<>(mTileSpecs);
- if (end) {
- newSpecs.add(spec);
- } else {
- newSpecs.add(0, spec);
- }
- changeTiles(mTileSpecs, newSpecs);
- }
+ addTile(spec, end ? POSITION_AT_END : 0);
}
- public void removeTile(ComponentName tile) {
- List<String> newSpecs = new ArrayList<>(mTileSpecs);
- newSpecs.remove(CustomTile.toSpec(tile));
- changeTiles(mTileSpecs, newSpecs);
+ /**
+ * This will call through {@link #changeTilesByUser}. It should only be used when a tile is
+ * removed by a <b>user action</b> like {@code adb}.
+ */
+ public void removeTileByUser(ComponentName tile) {
+ mMainExecutor.execute(() -> {
+ List<String> newSpecs = new ArrayList<>(mTileSpecs);
+ if (newSpecs.remove(CustomTile.toSpec(tile))) {
+ changeTilesByUser(mTileSpecs, newSpecs);
+ }
+ });
}
/**
* Change the tiles triggered by the user editing.
* <p>
* This is not called on device start, or on user change.
+ *
+ * {@link android.service.quicksettings.TileService#onTileRemoved} will be called for tiles
+ * that are removed.
*/
- public void changeTiles(List<String> previousTiles, List<String> newTiles) {
+ @MainThread
+ public void changeTilesByUser(List<String> previousTiles, List<String> newTiles) {
final List<String> copy = new ArrayList<>(previousTiles);
final int NP = copy.size();
for (int i = 0; i < NP; i++) {
diff --git a/packages/SystemUI/src/com/android/systemui/qs/customize/TileAdapter.java b/packages/SystemUI/src/com/android/systemui/qs/customize/TileAdapter.java
index e52bfbd..d84b12c 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/customize/TileAdapter.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/customize/TileAdapter.java
@@ -182,7 +182,7 @@
for (int i = 1; i < mTiles.size() && mTiles.get(i) != null; i++) {
newSpecs.add(mTiles.get(i).spec);
}
- host.changeTiles(mCurrentSpecs, newSpecs);
+ host.changeTilesByUser(mCurrentSpecs, newSpecs);
mCurrentSpecs = newSpecs;
}
@@ -200,7 +200,7 @@
/** */
public void resetTileSpecs(List<String> specs) {
// Notify the host so the tiles get removed callbacks.
- mHost.changeTiles(mCurrentSpecs, specs);
+ mHost.changeTilesByUser(mCurrentSpecs, specs);
setTileSpecs(specs);
}
diff --git a/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java b/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java
index bf565a8..cfc57db 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java
@@ -289,7 +289,7 @@
}
}
- mServices.getHost().removeTile(component);
+ mServices.getHost().removeTile(CustomTile.toSpec(component));
}
};
}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java
index 8782be5..9070ead 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java
@@ -428,7 +428,7 @@
if (isSafetyCenterEnabled && !mAutoTracker.isAdded(mSafetySpec)) {
initSafetyTile();
} else if (!isSafetyCenterEnabled && mAutoTracker.isAdded(mSafetySpec)) {
- mHost.removeTile(CustomTile.getComponentFromSpec(mSafetySpec));
+ mHost.removeTile(mSafetySpec);
mHost.unmarkTileAsAutoAdded(mSafetySpec);
}
}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java
index 9060d5f..ffd50ab 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java
@@ -187,7 +187,7 @@
public void remQsTile(ComponentName tile) {
QSPanelController qsPanelController = mCentralSurfaces.getQSPanelController();
if (qsPanelController != null && qsPanelController.getHost() != null) {
- qsPanelController.getHost().removeTile(tile);
+ qsPanelController.getHost().removeTileByUser(tile);
}
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSFragmentTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/QSFragmentTest.java
index 664af75..32c66d2 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSFragmentTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSFragmentTest.java
@@ -32,8 +32,6 @@
import android.content.Context;
import android.graphics.Rect;
import android.os.Bundle;
-import android.os.Handler;
-import android.os.Looper;
import android.testing.AndroidTestingRunner;
import android.testing.TestableLooper.RunWithLooper;
import android.view.LayoutInflater;
@@ -42,36 +40,23 @@
import androidx.test.filters.SmallTest;
-import com.android.internal.logging.UiEventLogger;
import com.android.keyguard.BouncerPanelExpansionCalculator;
import com.android.systemui.R;
import com.android.systemui.SysuiBaseFragmentTest;
import com.android.systemui.animation.ShadeInterpolation;
-import com.android.systemui.broadcast.BroadcastDispatcher;
import com.android.systemui.dump.DumpManager;
import com.android.systemui.media.MediaHost;
import com.android.systemui.plugins.FalsingManager;
import com.android.systemui.plugins.statusbar.StatusBarStateController;
import com.android.systemui.qs.customize.QSCustomizerController;
import com.android.systemui.qs.dagger.QSFragmentComponent;
-import com.android.systemui.qs.external.CustomTileStatePersister;
-import com.android.systemui.qs.external.TileLifecycleManager;
import com.android.systemui.qs.external.TileServiceRequestController;
-import com.android.systemui.qs.logging.QSLogger;
-import com.android.systemui.qs.tileimpl.QSFactoryImpl;
-import com.android.systemui.settings.UserTracker;
-import com.android.systemui.shared.plugins.PluginManager;
import com.android.systemui.statusbar.CommandQueue;
import com.android.systemui.statusbar.StatusBarState;
-import com.android.systemui.statusbar.phone.AutoTileManager;
-import com.android.systemui.statusbar.phone.CentralSurfaces;
import com.android.systemui.statusbar.phone.KeyguardBypassController;
-import com.android.systemui.statusbar.phone.StatusBarIconController;
import com.android.systemui.statusbar.policy.ConfigurationController;
import com.android.systemui.statusbar.policy.RemoteInputQuickSettingsDisabler;
-import com.android.systemui.tuner.TunerService;
import com.android.systemui.util.animation.UniqueObjectHostView;
-import com.android.systemui.util.settings.SecureSettings;
import org.junit.Before;
import org.junit.Test;
@@ -79,8 +64,6 @@
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
-import java.util.Optional;
-
@RunWith(AndroidTestingRunner.class)
@RunWithLooper(setAsMainLooper = true)
@SmallTest
@@ -125,34 +108,11 @@
mFragments.dispatchResume();
processAllMessages();
- QSTileHost host =
- new QSTileHost(
- mContext,
- mock(StatusBarIconController.class),
- mock(QSFactoryImpl.class),
- new Handler(),
- Looper.myLooper(),
- mock(PluginManager.class),
- mock(TunerService.class),
- () -> mock(AutoTileManager.class),
- mock(DumpManager.class),
- mock(BroadcastDispatcher.class),
- Optional.of(mock(CentralSurfaces.class)),
- mock(QSLogger.class),
- mock(UiEventLogger.class),
- mock(UserTracker.class),
- mock(SecureSettings.class),
- mock(CustomTileStatePersister.class),
- mTileServiceRequestControllerBuilder,
- mock(TileLifecycleManager.Factory.class));
-
qs.setListening(true);
processAllMessages();
qs.setListening(false);
processAllMessages();
- host.destroy();
- processAllMessages();
}
@Test
diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java
index 8cf3fe2..7dbc561 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java
@@ -32,12 +32,11 @@
import android.content.ComponentName;
import android.content.Context;
import android.content.Intent;
+import android.database.ContentObserver;
import android.os.Handler;
import android.os.Looper;
import android.os.UserHandle;
import android.testing.AndroidTestingRunner;
-import android.testing.TestableLooper;
-import android.testing.TestableLooper.RunWithLooper;
import android.view.View;
import androidx.annotation.Nullable;
@@ -48,7 +47,6 @@
import com.android.internal.util.CollectionUtils;
import com.android.systemui.R;
import com.android.systemui.SysuiTestCase;
-import com.android.systemui.broadcast.BroadcastDispatcher;
import com.android.systemui.classifier.FalsingManagerFake;
import com.android.systemui.dump.DumpManager;
import com.android.systemui.plugins.ActivityStarter;
@@ -68,8 +66,10 @@
import com.android.systemui.statusbar.phone.CentralSurfaces;
import com.android.systemui.statusbar.phone.StatusBarIconController;
import com.android.systemui.tuner.TunerService;
+import com.android.systemui.util.concurrency.FakeExecutor;
import com.android.systemui.util.settings.FakeSettings;
import com.android.systemui.util.settings.SecureSettings;
+import com.android.systemui.util.time.FakeSystemClock;
import org.junit.Before;
import org.junit.Test;
@@ -81,18 +81,19 @@
import java.io.StringWriter;
import java.util.List;
import java.util.Optional;
+import java.util.concurrent.Executor;
import javax.inject.Provider;
@RunWith(AndroidTestingRunner.class)
@SmallTest
-@RunWithLooper(setAsMainLooper = true)
public class QSTileHostTest extends SysuiTestCase {
private static String MOCK_STATE_STRING = "MockState";
private static ComponentName CUSTOM_TILE =
ComponentName.unflattenFromString("TEST_PKG/.TEST_CLS");
private static final String CUSTOM_TILE_SPEC = CustomTile.toSpec(CUSTOM_TILE);
+ private static final String SETTING = QSTileHost.TILES_SETTING;
@Mock
private StatusBarIconController mIconController;
@@ -107,8 +108,6 @@
@Mock
private DumpManager mDumpManager;
@Mock
- private BroadcastDispatcher mBroadcastDispatcher;
- @Mock
private QSTile.State mMockState;
@Mock
private CentralSurfaces mCentralSurfaces;
@@ -132,31 +131,47 @@
@Mock
private TileLifecycleManager mTileLifecycleManager;
- private Handler mHandler;
- private TestableLooper mLooper;
+ private FakeExecutor mMainExecutor;
+
private QSTileHost mQSTileHost;
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
- mLooper = TestableLooper.get(this);
- mHandler = new Handler(mLooper.getLooper());
+ mMainExecutor = new FakeExecutor(new FakeSystemClock());
+
when(mTileServiceRequestControllerBuilder.create(any()))
.thenReturn(mTileServiceRequestController);
when(mTileLifecycleManagerFactory.create(any(Intent.class), any(UserHandle.class)))
.thenReturn(mTileLifecycleManager);
mSecureSettings = new FakeSettings();
- mSecureSettings.putStringForUser(
- QSTileHost.TILES_SETTING, "", "", false, mUserTracker.getUserId(), false);
- mQSTileHost = new TestQSTileHost(mContext, mIconController, mDefaultFactory, mHandler,
- mLooper.getLooper(), mPluginManager, mTunerService, mAutoTiles, mDumpManager,
- mBroadcastDispatcher, mCentralSurfaces, mQSLogger, mUiEventLogger, mUserTracker,
- mSecureSettings, mCustomTileStatePersister, mTileServiceRequestControllerBuilder,
- mTileLifecycleManagerFactory);
+ saveSetting("");
+ mQSTileHost = new TestQSTileHost(mContext, mIconController, mDefaultFactory, mMainExecutor,
+ mPluginManager, mTunerService, mAutoTiles, mDumpManager, mCentralSurfaces,
+ mQSLogger, mUiEventLogger, mUserTracker, mSecureSettings, mCustomTileStatePersister,
+ mTileServiceRequestControllerBuilder, mTileLifecycleManagerFactory);
+
+ mSecureSettings.registerContentObserverForUser(SETTING, new ContentObserver(null) {
+ @Override
+ public void onChange(boolean selfChange) {
+ super.onChange(selfChange);
+ mMainExecutor.execute(() -> mQSTileHost.onTuningChanged(SETTING, getSetting()));
+ mMainExecutor.runAllReady();
+ }
+ }, mUserTracker.getUserId());
setUpTileFactory();
}
+ private void saveSetting(String value) {
+ mSecureSettings.putStringForUser(
+ SETTING, value, "", false, mUserTracker.getUserId(), false);
+ }
+
+ private String getSetting() {
+ return mSecureSettings.getStringForUser(SETTING, mUserTracker.getUserId());
+ }
+
private void setUpTileFactory() {
when(mMockState.toString()).thenReturn(MOCK_STATE_STRING);
// Only create this kind of tiles
@@ -173,6 +188,10 @@
return new NotAvailableTile(mQSTileHost);
} else if (CUSTOM_TILE_SPEC.equals(spec)) {
return mCustomTile;
+ } else if ("internet".equals(spec)
+ || "wifi".equals(spec)
+ || "cell".equals(spec)) {
+ return new TestTile1(mQSTileHost);
} else {
return null;
}
@@ -196,14 +215,14 @@
public void testInvalidSpecUsesDefault() {
mContext.getOrCreateTestableResources()
.addOverride(R.string.quick_settings_tiles, "spec1,spec2");
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "not-valid");
+ saveSetting("not-valid");
assertEquals(2, mQSTileHost.getTiles().size());
}
@Test
public void testRemoveWifiAndCellularWithoutInternet() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "wifi, spec1, cell, spec2");
+ saveSetting("wifi, spec1, cell, spec2");
assertEquals("internet", mQSTileHost.mTileSpecs.get(0));
assertEquals("spec1", mQSTileHost.mTileSpecs.get(1));
@@ -212,7 +231,7 @@
@Test
public void testRemoveWifiAndCellularWithInternet() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "wifi, spec1, cell, spec2, internet");
+ saveSetting("wifi, spec1, cell, spec2, internet");
assertEquals("spec1", mQSTileHost.mTileSpecs.get(0));
assertEquals("spec2", mQSTileHost.mTileSpecs.get(1));
@@ -221,7 +240,7 @@
@Test
public void testRemoveWifiWithoutInternet() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1, wifi, spec2");
+ saveSetting("spec1, wifi, spec2");
assertEquals("spec1", mQSTileHost.mTileSpecs.get(0));
assertEquals("internet", mQSTileHost.mTileSpecs.get(1));
@@ -230,7 +249,7 @@
@Test
public void testRemoveCellWithInternet() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1, spec2, cell, internet");
+ saveSetting("spec1, spec2, cell, internet");
assertEquals("spec1", mQSTileHost.mTileSpecs.get(0));
assertEquals("spec2", mQSTileHost.mTileSpecs.get(1));
@@ -239,7 +258,7 @@
@Test
public void testNoWifiNoCellularNoInternet() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec2");
+ saveSetting("spec1,spec2");
assertEquals("spec1", mQSTileHost.mTileSpecs.get(0));
assertEquals("spec2", mQSTileHost.mTileSpecs.get(1));
@@ -249,7 +268,7 @@
public void testSpecWithInvalidDoesNotUseDefault() {
mContext.getOrCreateTestableResources()
.addOverride(R.string.quick_settings_tiles, "spec1,spec2");
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec2,not-valid");
+ saveSetting("spec2,not-valid");
assertEquals(1, mQSTileHost.getTiles().size());
QSTile element = CollectionUtils.firstOrNull(mQSTileHost.getTiles());
@@ -258,7 +277,7 @@
@Test
public void testDump() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec2");
+ saveSetting("spec1,spec2");
StringWriter w = new StringWriter();
PrintWriter pw = new PrintWriter(w);
mQSTileHost.dump(pw, new String[]{});
@@ -274,7 +293,7 @@
public void testDefault() {
mContext.getOrCreateTestableResources()
.addOverride(R.string.quick_settings_tiles_default, "spec1");
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "default");
+ saveSetting("default");
assertEquals(1, mQSTileHost.getTiles().size());
QSTile element = CollectionUtils.firstOrNull(mQSTileHost.getTiles());
assertTrue(element instanceof TestTile1);
@@ -285,7 +304,7 @@
public void testNoRepeatedSpecs_addTile() {
mContext.getOrCreateTestableResources()
.addOverride(R.string.quick_settings_tiles, "spec1,spec2");
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec2");
+ saveSetting("spec1,spec2");
mQSTileHost.addTile("spec1");
@@ -298,9 +317,10 @@
public void testAddTileAtValidPosition() {
mContext.getOrCreateTestableResources()
.addOverride(R.string.quick_settings_tiles, "spec1,spec3");
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec3");
+ saveSetting("spec1,spec3");
mQSTileHost.addTile("spec2", 1);
+ mMainExecutor.runAllReady();
assertEquals(3, mQSTileHost.mTileSpecs.size());
assertEquals("spec1", mQSTileHost.mTileSpecs.get(0));
@@ -312,9 +332,10 @@
public void testAddTileAtInvalidPositionAddsToEnd() {
mContext.getOrCreateTestableResources()
.addOverride(R.string.quick_settings_tiles, "spec1,spec3");
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec3");
+ saveSetting("spec1,spec3");
mQSTileHost.addTile("spec2", 100);
+ mMainExecutor.runAllReady();
assertEquals(3, mQSTileHost.mTileSpecs.size());
assertEquals("spec1", mQSTileHost.mTileSpecs.get(0));
@@ -326,9 +347,10 @@
public void testAddTileAtEnd() {
mContext.getOrCreateTestableResources()
.addOverride(R.string.quick_settings_tiles, "spec1,spec3");
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec3");
+ saveSetting("spec1,spec3");
mQSTileHost.addTile("spec2", QSTileHost.POSITION_AT_END);
+ mMainExecutor.runAllReady();
assertEquals(3, mQSTileHost.mTileSpecs.size());
assertEquals("spec1", mQSTileHost.mTileSpecs.get(0));
@@ -338,9 +360,10 @@
@Test
public void testNoRepeatedSpecs_customTile() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, CUSTOM_TILE_SPEC);
+ saveSetting(CUSTOM_TILE_SPEC);
mQSTileHost.addTile(CUSTOM_TILE, /* end */ false);
+ mMainExecutor.runAllReady();
assertEquals(1, mQSTileHost.mTileSpecs.size());
assertEquals(CUSTOM_TILE_SPEC, mQSTileHost.mTileSpecs.get(0));
@@ -348,9 +371,10 @@
@Test
public void testAddedAtBeginningOnDefault_customTile() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1"); // seed
+ saveSetting("spec1"); // seed
mQSTileHost.addTile(CUSTOM_TILE);
+ mMainExecutor.runAllReady();
assertEquals(2, mQSTileHost.mTileSpecs.size());
assertEquals(CUSTOM_TILE_SPEC, mQSTileHost.mTileSpecs.get(0));
@@ -358,9 +382,10 @@
@Test
public void testAddedAtBeginning_customTile() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1"); // seed
+ saveSetting("spec1"); // seed
mQSTileHost.addTile(CUSTOM_TILE, /* end */ false);
+ mMainExecutor.runAllReady();
assertEquals(2, mQSTileHost.mTileSpecs.size());
assertEquals(CUSTOM_TILE_SPEC, mQSTileHost.mTileSpecs.get(0));
@@ -368,9 +393,10 @@
@Test
public void testAddedAtEnd_customTile() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1"); // seed
+ saveSetting("spec1"); // seed
mQSTileHost.addTile(CUSTOM_TILE, /* end */ true);
+ mMainExecutor.runAllReady();
assertEquals(2, mQSTileHost.mTileSpecs.size());
assertEquals(CUSTOM_TILE_SPEC, mQSTileHost.mTileSpecs.get(1));
@@ -409,13 +435,13 @@
@Test
public void testNotAvailableTile_specNotNull() {
- mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "na");
+ saveSetting("na");
verify(mQSLogger, never()).logTileDestroyed(isNull(), anyString());
}
@Test
public void testCustomTileRemoved_stateDeleted() {
- mQSTileHost.changeTiles(List.of(CUSTOM_TILE_SPEC), List.of());
+ mQSTileHost.changeTilesByUser(List.of(CUSTOM_TILE_SPEC), List.of());
verify(mCustomTileStatePersister)
.removeState(new TileServiceKey(CUSTOM_TILE, mQSTileHost.getUserId()));
@@ -423,29 +449,99 @@
@Test
public void testRemoveTiles() {
- List<String> tiles = List.of("spec1", "spec2", "spec3");
- mQSTileHost.saveTilesToSettings(tiles);
+ saveSetting("spec1,spec2,spec3");
mQSTileHost.removeTiles(List.of("spec1", "spec2"));
+ mMainExecutor.runAllReady();
assertEquals(List.of("spec3"), mQSTileHost.mTileSpecs);
}
+ @Test
+ public void testTilesRemovedInQuickSuccession() {
+ saveSetting("spec1,spec2,spec3");
+ mQSTileHost.removeTile("spec1");
+ mQSTileHost.removeTile("spec3");
+
+ mMainExecutor.runAllReady();
+ assertEquals(List.of("spec2"), mQSTileHost.mTileSpecs);
+ assertEquals("spec2", getSetting());
+ }
+
+ @Test
+ public void testAddTileInMainThread() {
+ saveSetting("spec1,spec2");
+
+ mQSTileHost.addTile("spec3");
+ assertEquals(List.of("spec1", "spec2"), mQSTileHost.mTileSpecs);
+
+ mMainExecutor.runAllReady();
+ assertEquals(List.of("spec1", "spec2", "spec3"), mQSTileHost.mTileSpecs);
+ }
+
+ @Test
+ public void testRemoveTileInMainThread() {
+ saveSetting("spec1,spec2");
+
+ mQSTileHost.removeTile("spec1");
+ assertEquals(List.of("spec1", "spec2"), mQSTileHost.mTileSpecs);
+
+ mMainExecutor.runAllReady();
+ assertEquals(List.of("spec2"), mQSTileHost.mTileSpecs);
+ }
+
+ @Test
+ public void testRemoveTilesInMainThread() {
+ saveSetting("spec1,spec2,spec3");
+
+ mQSTileHost.removeTiles(List.of("spec3", "spec1"));
+ assertEquals(List.of("spec1", "spec2", "spec3"), mQSTileHost.mTileSpecs);
+
+ mMainExecutor.runAllReady();
+ assertEquals(List.of("spec2"), mQSTileHost.mTileSpecs);
+ }
+
+ @Test
+ public void testRemoveTileByUserInMainThread() {
+ saveSetting("spec1," + CUSTOM_TILE_SPEC);
+
+ mQSTileHost.removeTileByUser(CUSTOM_TILE);
+ assertEquals(List.of("spec1", CUSTOM_TILE_SPEC), mQSTileHost.mTileSpecs);
+
+ mMainExecutor.runAllReady();
+ assertEquals(List.of("spec1"), mQSTileHost.mTileSpecs);
+ }
+
+ @Test
+ public void testNonValidTileNotStoredInSettings() {
+ saveSetting("spec1,not-valid");
+
+ assertEquals(List.of("spec1"), mQSTileHost.mTileSpecs);
+ assertEquals("spec1", getSetting());
+ }
+
+ @Test
+ public void testNotAvailableTileNotStoredInSettings() {
+ saveSetting("spec1,na");
+
+ assertEquals(List.of("spec1"), mQSTileHost.mTileSpecs);
+ assertEquals("spec1", getSetting());
+ }
+
private class TestQSTileHost extends QSTileHost {
TestQSTileHost(Context context, StatusBarIconController iconController,
- QSFactory defaultFactory, Handler mainHandler, Looper bgLooper,
+ QSFactory defaultFactory, Executor mainExecutor,
PluginManager pluginManager, TunerService tunerService,
Provider<AutoTileManager> autoTiles, DumpManager dumpManager,
- BroadcastDispatcher broadcastDispatcher, CentralSurfaces centralSurfaces,
- QSLogger qsLogger, UiEventLogger uiEventLogger, UserTracker userTracker,
- SecureSettings secureSettings, CustomTileStatePersister customTileStatePersister,
+ CentralSurfaces centralSurfaces, QSLogger qsLogger, UiEventLogger uiEventLogger,
+ UserTracker userTracker, SecureSettings secureSettings,
+ CustomTileStatePersister customTileStatePersister,
TileServiceRequestController.Builder tileServiceRequestControllerBuilder,
TileLifecycleManager.Factory tileLifecycleManagerFactory) {
- super(context, iconController, defaultFactory, mainHandler, bgLooper, pluginManager,
- tunerService, autoTiles, dumpManager, broadcastDispatcher,
- Optional.of(centralSurfaces), qsLogger, uiEventLogger, userTracker,
- secureSettings, customTileStatePersister, tileServiceRequestControllerBuilder,
- tileLifecycleManagerFactory);
+ super(context, iconController, defaultFactory, mainExecutor, pluginManager,
+ tunerService, autoTiles, dumpManager, Optional.of(centralSurfaces), qsLogger,
+ uiEventLogger, userTracker, secureSettings, customTileStatePersister,
+ tileServiceRequestControllerBuilder, tileLifecycleManagerFactory);
}
@Override
@@ -455,25 +551,16 @@
@Override
public void onPluginDisconnected(QSFactory plugin) {
}
-
- @Override
- void saveTilesToSettings(List<String> tileSpecs) {
- super.saveTilesToSettings(tileSpecs);
- // After tiles are changed, make sure to call onTuningChanged with the new setting if it
- // changed
- String specs = mSecureSettings.getStringForUser(
- QSTileHost.TILES_SETTING, mUserTracker.getUserId());
- onTuningChanged(TILES_SETTING, specs);
- }
}
+
private class TestTile extends QSTileImpl<QSTile.State> {
protected TestTile(QSHost host) {
super(
host,
- mLooper.getLooper(),
- new Handler(mLooper.getLooper()),
+ mock(Looper.class),
+ mock(Handler.class),
new FalsingManagerFake(),
mock(MetricsLogger.class),
mock(StatusBarStateController.class),
diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileAdapterTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileAdapterTest.java
index 3d53062..d42cbe3 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileAdapterTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileAdapterTest.java
@@ -55,6 +55,6 @@
@Test
public void testResetNotifiesHost() {
mTileAdapter.resetTileSpecs(Collections.emptyList());
- verify(mQSTileHost).changeTiles(any(), any());
+ verify(mQSTileHost).changeTilesByUser(any(), any());
}
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServicesTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServicesTest.java
index 6b7e5b93..471ddfd 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServicesTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServicesTest.java
@@ -29,6 +29,7 @@
import android.content.ComponentName;
import android.content.Intent;
import android.os.Handler;
+import android.os.HandlerExecutor;
import android.os.RemoteException;
import android.os.UserHandle;
import android.service.quicksettings.IQSTileService;
@@ -65,6 +66,7 @@
import java.util.ArrayList;
import java.util.Optional;
+import java.util.concurrent.Executor;
import javax.inject.Provider;
@@ -130,17 +132,16 @@
.thenReturn(mTileLifecycleManager);
Provider<Handler> provider = () -> new Handler(mTestableLooper.getLooper());
+ Executor executor = new HandlerExecutor(provider.get());
QSTileHost host = new QSTileHost(mContext,
mStatusBarIconController,
mQSFactory,
- provider.get(),
- mTestableLooper.getLooper(),
+ executor,
mPluginManager,
mTunerService,
() -> mAutoTileManager,
mDumpManager,
- mock(BroadcastDispatcher.class),
Optional.of(mCentralSurfaces),
mQSLogger,
mUiEventLogger,
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/AutoTileManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/AutoTileManagerTest.java
index 371119c..4ccbc6d 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/AutoTileManagerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/AutoTileManagerTest.java
@@ -490,7 +490,7 @@
mAutoTileManager.init();
when(mAutoAddTracker.isAdded(TEST_CUSTOM_SAFETY_SPEC)).thenReturn(true);
mAutoTileManager.mSafetyCallback.onSafetyCenterEnableChanged(false);
- verify(mQsTileHost, times(1)).removeTile(safetyComponent);
+ verify(mQsTileHost, times(1)).removeTile(TEST_CUSTOM_SAFETY_SPEC);
}
@Test