Merge changes from topic "race_condition_tm-d1-dev" into tm-d1-dev

* changes:
  Reduce blocking calls to Settings in main thread
  Fix race condition in QSTileHost

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
@@ -105,6 +110,11 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, D
     private Context mUserContext;
     private UserTracker mUserTracker;
     private SecureSettings mSecureSettings;
+    // Keep track of whether mTilesList contains the same information as the Settings value.
+    // This is a performance optimization to reduce the number of blocking calls to Settings from
+    // main thread.
+    // This is enforced by only cleaning the flag at the end of a successful run of #onTuningChanged
+    private boolean mTilesListDirty = true;
 
     private final TileServiceRequestController mTileServiceRequestController;
     private TileLifecycleManager.Factory mTileLifeCycleManagerFactory;
@@ -113,13 +123,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 +145,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 +159,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 +266,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 +365,45 @@ 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);
+            }
+            mTilesListDirty = false;
             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,31 +427,41 @@ 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");
+    // When calling this, you may want to modify mTilesListDirty accordingly.
+    @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;
-        final List<String> tileSpecs = loadTileSpecs(mContext, setting);
+        if (!mTilesListDirty) {
+            tileSpecs = new ArrayList<>(mTileSpecs);
+        } else {
+            tileSpecs = loadTileSpecs(mContext,
+                    mSecureSettings.getStringForUser(TILES_SETTING, mCurrentUser));
+        }
         if (changeFunction.test(tileSpecs)) {
+            mTilesListDirty = true;
             saveTilesToSettings(tileSpecs);
         }
     }
@@ -421,29 +477,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++) {
@@ -462,6 +521,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener<QSFactory>, D
             }
         }
         if (DEBUG) Log.d(TAG, "saveCurrentTiles " + newTiles);
+        mTilesListDirty = true;
         saveTilesToSettings(newTiles);
     }
 

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