Fix race condition in QSTileHost

QSTileHost had three problems that caused a race condition:
 * Calls to add/remove tiles could come and be executed in any thread.
 * Different paths to add/remove tiles used different ground truths on
   what tiles were currently active.
 * The list of tiles contained in mTileSpecs was not necessarily the
   same as the visible tiles.

This caused the following problem:
 * If two tiles (usually CustomTile) were removed roughly at the same
   time, only one of them would actually be removed from the Setting, as
   both of them would race to remove from the same list. This caused...
 * There would be leftover tiles that cannot connect to a TileService
   and therefore show empty UI.

The solution is as follows:
 * Make sure that all operations that modify the tiles are performed in
   the main thread.
 * Make sure that all operations that modify the tiles use the same
   truth for the current state.
 * Make sure that mTileSpecs always reflects the setting at the end of
   onTuningChanged. This means that tiles that are destroyed or removed
   will never be in this list.

Test: atest com.android.systemui.qs
Test: manual, start Guest user and notice that there are no blank tiles.
Fixes: 235561921
Change-Id: I6ffbb7510e6854f18bc427d1ae1e782918c799dc
Merged-In: I6ffbb7510e6854f18bc427d1ae1e782918c799dc
(cherry picked from commit 7f2bb15f4e)

packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java

@@ -19,8 +19,6 @@ import android.content.Context;
 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.text.TextUtils;
 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.InstanceIdSequence;
 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.LinkedHashMap;
 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 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, D
     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 class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, D
     public QSTileHost(Context context,
             StatusBarIconController iconController,
             QSFactory defaultFactory,
-            @Main Handler mainHandler,
+            @Main Executor mainExecutor,
-            @Background Looper bgLooper,
             PluginManager pluginManager,
             TunerService tunerService,
             Provider<AutoTileManager> autoTiles,
             DumpManager dumpManager,
-            BroadcastDispatcher broadcastDispatcher,
             Optional<CentralSurfaces> centralSurfacesOptional,
             QSLogger qsLogger,
             UiEventLogger uiEventLogger,
@@ -137,7 +140,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, D
         mDumpManager = dumpManager;
         mQSLogger = qsLogger;
         mUiEventLogger = uiEventLogger;
-        mBroadcastDispatcher = broadcastDispatcher;
+        mMainExecutor = mainExecutor;
         mTileServiceRequestController = tileServiceRequestControllerBuilder.create(this);
         mTileLifeCycleManagerFactory = tileLifecycleManagerFactory;
 
@@ -151,7 +154,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, D
         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 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, D
         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 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, D
         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 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, D
      * @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");
+        mMainExecutor.execute(() ->
-        changeTileSpecs(tileSpecs -> {
+                changeTileSpecs(tileSpecs -> {
-            if (tileSpecs.contains(spec)) return false;
+                    if (tileSpecs.contains(spec)) return false;
 
-            int size = tileSpecs.size();
+                    int size = tileSpecs.size();
-            if (requestPosition == POSITION_AT_END || requestPosition >= size) {
+                    if (requestPosition == POSITION_AT_END || requestPosition >= size) {
-                tileSpecs.add(spec);
+                        tileSpecs.add(spec);
-            } else {
+                    } else {
-                tileSpecs.add(requestPosition, spec);
+                        tileSpecs.add(requestPosition, spec);
-            }
+                    }
-            return true;
+                    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 class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, D
      */
     public void addTile(ComponentName tile, boolean end) {
         String spec = CustomTile.toSpec(tile);
-        if (!mTileSpecs.contains(spec)) {
+        addTile(spec, end ? POSITION_AT_END : 0);
-            List<String> newSpecs = new ArrayList<>(mTileSpecs);
-            if (end) {
-                newSpecs.add(spec);
-            } else {
-                newSpecs.add(0, spec);
-            }
-            changeTiles(mTileSpecs, newSpecs);
-        }
     }
 
-    public void removeTile(ComponentName tile) {
+    /**
-        List<String> newSpecs = new ArrayList<>(mTileSpecs);
+     * This will call through {@link #changeTilesByUser}. It should only be used when a tile is
-        newSpecs.remove(CustomTile.toSpec(tile));
+     * removed by a <b>user action</b> like {@code adb}.
-        changeTiles(mTileSpecs, newSpecs);
+     */
+    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++) {

packages/SystemUI/src/com/android/systemui/qs/customize/TileAdapter.java

@@ -182,7 +182,7 @@ public class TileAdapter extends RecyclerView.Adapter<Holder> implements TileSta
         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 class TileAdapter extends RecyclerView.Adapter<Holder> implements TileSta
     /** */
     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);
     }
 

packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java

@@ -289,7 +289,7 @@ public class TileServiceManager {
                 }
             }
 
-            mServices.getHost().removeTile(component);
+            mServices.getHost().removeTile(CustomTile.toSpec(component));
         }
     };
 }

packages/SystemUI/src/com/android/systemui/statusbar/phone/AutoTileManager.java

@@ -428,7 +428,7 @@ public class AutoTileManager implements UserAwareController {
             if (isSafetyCenterEnabled && !mAutoTracker.isAdded(mSafetySpec)) {
                 initSafetyTile();
             } else if (!isSafetyCenterEnabled && mAutoTracker.isAdded(mSafetySpec)) {
-                mHost.removeTile(CustomTile.getComponentFromSpec(mSafetySpec));
+                mHost.removeTile(mSafetySpec);
                 mHost.unmarkTileAsAutoAdded(mSafetySpec);
             }
         }

packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java

@@ -187,7 +187,7 @@ public class CentralSurfacesCommandQueueCallbacks implements CommandQueue.Callba
     public void remQsTile(ComponentName tile) {
         QSPanelController qsPanelController = mCentralSurfaces.getQSPanelController();
         if (qsPanelController != null && qsPanelController.getHost() != null) {
-            qsPanelController.getHost().removeTile(tile);
+            qsPanelController.getHost().removeTileByUser(tile);
         }
     }
 

packages/SystemUI/tests/src/com/android/systemui/qs/QSFragmentTest.java

@@ -32,8 +32,6 @@ import android.app.Fragment;
 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 android.view.ViewGroup;
 
 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.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
-import java.util.Optional;
-
 @RunWith(AndroidTestingRunner.class)
 @RunWithLooper(setAsMainLooper = true)
 @SmallTest
@@ -125,34 +108,11 @@ public class QSFragmentTest extends SysuiBaseFragmentTest {
         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

packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java

@@ -32,12 +32,11 @@ import static org.mockito.Mockito.when;
 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.logging.UiEventLogger;
 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.AutoTileManager;
 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.PrintWriter;
 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 @@ public class QSTileHostTest extends SysuiTestCase {
     @Mock
     private DumpManager mDumpManager;
     @Mock
-    private BroadcastDispatcher mBroadcastDispatcher;
-    @Mock
     private QSTile.State mMockState;
     @Mock
     private CentralSurfaces mCentralSurfaces;
@@ -132,31 +131,47 @@ public class QSTileHostTest extends SysuiTestCase {
     @Mock
     private TileLifecycleManager mTileLifecycleManager;
 
-    private Handler mHandler;
+    private FakeExecutor mMainExecutor;
-    private TestableLooper mLooper;
+
     private QSTileHost mQSTileHost;
 
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
-        mLooper = TestableLooper.get(this);
+        mMainExecutor = new FakeExecutor(new FakeSystemClock());
-        mHandler = new Handler(mLooper.getLooper());
+
         when(mTileServiceRequestControllerBuilder.create(any()))
                 .thenReturn(mTileServiceRequestController);
         when(mTileLifecycleManagerFactory.create(any(Intent.class), any(UserHandle.class)))
                 .thenReturn(mTileLifecycleManager);
 
         mSecureSettings = new FakeSettings();
-        mSecureSettings.putStringForUser(
+        saveSetting("");
-                QSTileHost.TILES_SETTING, "", "", false, mUserTracker.getUserId(), false);
+        mQSTileHost = new TestQSTileHost(mContext, mIconController, mDefaultFactory, mMainExecutor,
-        mQSTileHost = new TestQSTileHost(mContext, mIconController, mDefaultFactory, mHandler,
+                mPluginManager, mTunerService, mAutoTiles, mDumpManager, mCentralSurfaces,
-                mLooper.getLooper(), mPluginManager, mTunerService, mAutoTiles, mDumpManager,
+                mQSLogger, mUiEventLogger, mUserTracker, mSecureSettings, mCustomTileStatePersister,
-                mBroadcastDispatcher, mCentralSurfaces, mQSLogger, mUiEventLogger, mUserTracker,
+                mTileServiceRequestControllerBuilder, mTileLifecycleManagerFactory);
-                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 @@ public class QSTileHostTest extends SysuiTestCase {
                         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 class QSTileHostTest extends SysuiTestCase {
     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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @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 class QSTileHostTest extends SysuiTestCase {
     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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @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 class QSTileHostTest extends SysuiTestCase {
     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 class QSTileHostTest extends SysuiTestCase {
     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 class QSTileHostTest extends SysuiTestCase {
     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 class QSTileHostTest extends SysuiTestCase {
     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 class QSTileHostTest extends SysuiTestCase {
     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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @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 @@ public class QSTileHostTest extends SysuiTestCase {
 
     @Test
     public void testRemoveTiles() {
-        List<String> tiles = List.of("spec1", "spec2", "spec3");
+        saveSetting("spec1,spec2,spec3");
-        mQSTileHost.saveTilesToSettings(tiles);
 
         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,
+                CentralSurfaces centralSurfaces, QSLogger qsLogger, UiEventLogger uiEventLogger,
-                QSLogger qsLogger, UiEventLogger uiEventLogger, UserTracker userTracker,
+                UserTracker userTracker, SecureSettings secureSettings,
-                SecureSettings secureSettings, CustomTileStatePersister customTileStatePersister,
+                CustomTileStatePersister customTileStatePersister,
                 TileServiceRequestController.Builder tileServiceRequestControllerBuilder,
                 TileLifecycleManager.Factory tileLifecycleManagerFactory) {
-            super(context, iconController, defaultFactory, mainHandler, bgLooper, pluginManager,
+            super(context, iconController, defaultFactory, mainExecutor, pluginManager,
-                    tunerService, autoTiles, dumpManager, broadcastDispatcher,
+                    tunerService, autoTiles, dumpManager, Optional.of(centralSurfaces), qsLogger,
-                    Optional.of(centralSurfaces), qsLogger, uiEventLogger, userTracker,
+                    uiEventLogger, userTracker, secureSettings, customTileStatePersister,
-                    secureSettings, customTileStatePersister, tileServiceRequestControllerBuilder,
+                    tileServiceRequestControllerBuilder, tileLifecycleManagerFactory);
-                    tileLifecycleManagerFactory);
         }
 
         @Override
@@ -455,25 +551,16 @@ public class QSTileHostTest extends SysuiTestCase {
         @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(),
+                    mock(Looper.class),
-                    new Handler(mLooper.getLooper()),
+                    mock(Handler.class),
                     new FalsingManagerFake(),
                     mock(MetricsLogger.class),
                     mock(StatusBarStateController.class),

packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileAdapterTest.java

@@ -55,6 +55,6 @@ public class TileAdapterTest extends SysuiTestCase {
     @Test
     public void testResetNotifiesHost() {
         mTileAdapter.resetTileSpecs(Collections.emptyList());
-        verify(mQSTileHost).changeTiles(any(), any());
+        verify(mQSTileHost).changeTilesByUser(any(), any());
     }
 }

packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServicesTest.java

@@ -29,6 +29,7 @@ import static org.mockito.Mockito.when;
 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 org.mockito.MockitoAnnotations;
 
 import java.util.ArrayList;
 import java.util.Optional;
+import java.util.concurrent.Executor;
 
 import javax.inject.Provider;
 
@@ -130,17 +132,16 @@ public class TileServicesTest extends SysuiTestCase {
                 .thenReturn(mTileLifecycleManager);
 
         Provider<Handler> provider = () -> new Handler(mTestableLooper.getLooper());
+        Executor executor = new HandlerExecutor(provider.get());
 
         QSTileHost host = new QSTileHost(mContext,
                 mStatusBarIconController,
                 mQSFactory,
-                provider.get(),
+                executor,
-                mTestableLooper.getLooper(),
                 mPluginManager,
                 mTunerService,
                 () -> mAutoTileManager,
                 mDumpManager,
-                mock(BroadcastDispatcher.class),
                 Optional.of(mCentralSurfaces),
                 mQSLogger,
                 mUiEventLogger,

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/AutoTileManagerTest.java

@@ -490,7 +490,7 @@ public class AutoTileManagerTest extends SysuiTestCase {
         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