Ensure TileQueryHelper#mTiles is thread safe.

- Ensure TileQueryHelper#mTiles is only modified on the background
thread and a send a copy to listeners.
- Do not call onTilesChanged until all stock tiles are loaded (as
opposed to after each tile) to reduce tile spec recalculation.
- Remove completion callback and instead maintain query state in
TileQueryHelper

Bug: 75415415
Test: manually
Change-Id: I2a20bd916dae0ee9f4c422ec4e2ac3f2d47da3dc

diff --git a/packages/SystemUI/src/com/android/systemui/qs/customize/QSCustomizer.java b/packages/SystemUI/src/com/android/systemui/qs/customize/QSCustomizer.java
index 3847040..a3d6c6c 100644 (file)
--- a/packages/SystemUI/src/com/android/systemui/qs/customize/QSCustomizer.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/customize/QSCustomizer.java
@@ -18,14 +18,9 @@ package com.android.systemui.qs.customize;
 import android.animation.Animator;
 import android.animation.Animator.AnimatorListener;
 import android.animation.AnimatorListenerAdapter;
-import android.app.AlertDialog;
 import android.content.Context;
 import android.content.res.Configuration;
-import android.graphics.drawable.Drawable;
-import android.graphics.drawable.TransitionDrawable;
 import android.os.Bundle;
-import android.os.Handler;
-import android.os.Looper;
 import android.support.v7.widget.DefaultItemAnimator;
 import android.support.v7.widget.GridLayoutManager;
 import android.support.v7.widget.RecyclerView;
@@ -36,8 +31,6 @@ import android.view.LayoutInflater;
 import android.view.Menu;
 import android.view.MenuItem;
 import android.view.View;
-import android.view.WindowManager;
-import android.view.WindowManager.LayoutParams;
 import android.widget.LinearLayout;
 import android.widget.Toolbar;
 import android.widget.Toolbar.OnMenuItemClickListener;
@@ -48,12 +41,10 @@ import com.android.systemui.Dependency;
 import com.android.systemui.R;
 import com.android.systemui.plugins.qs.QS;
 import com.android.systemui.plugins.qs.QSTile;
-import com.android.systemui.qs.QSContainerImpl;
 import com.android.systemui.qs.QSDetailClipper;
 import com.android.systemui.qs.QSTileHost;
 import com.android.systemui.statusbar.phone.LightBarController;
 import com.android.systemui.statusbar.phone.NotificationsQuickSettingsContainer;
-import com.android.systemui.statusbar.phone.SystemUIDialog;
 import com.android.systemui.statusbar.policy.KeyguardMonitor;
 import com.android.systemui.statusbar.policy.KeyguardMonitor.Callback;
 
@@ -73,6 +64,7 @@ public class QSCustomizer extends LinearLayout implements OnMenuItemClickListene
 
     private final QSDetailClipper mClipper;
     private final LightBarController mLightBarController;
+    private final TileQueryHelper mTileQueryHelper;
 
     private boolean isShown;
     private QSTileHost mHost;
@@ -82,7 +74,6 @@ public class QSCustomizer extends LinearLayout implements OnMenuItemClickListene
     private boolean mCustomizing;
     private NotificationsQuickSettingsContainer mNotifQsContainer;
     private QS mQs;
-    private boolean mFinishedFetchingTiles = false;
     private int mX;
     private int mY;
     private boolean mOpening;
@@ -112,6 +103,7 @@ public class QSCustomizer extends LinearLayout implements OnMenuItemClickListene
 
         mRecyclerView = findViewById(android.R.id.list);
         mTileAdapter = new TileAdapter(getContext());
+        mTileQueryHelper = new TileQueryHelper(context, mTileAdapter);
         mRecyclerView.setAdapter(mTileAdapter);
         mTileAdapter.getItemTouchHelper().attachToRecyclerView(mRecyclerView);
         GridLayoutManager layout = new GridLayoutManager(getContext(), 3);
@@ -193,12 +185,7 @@ public class QSCustomizer extends LinearLayout implements OnMenuItemClickListene
     }
 
     private void queryTiles() {
-        mFinishedFetchingTiles = false;
-        Runnable tileQueryFetchCompletion = () -> {
-            Handler mainHandler = new Handler(Looper.getMainLooper());
-            mainHandler.post(() -> mFinishedFetchingTiles = true);
-        };
-        new TileQueryHelper(mContext, mHost, mTileAdapter, tileQueryFetchCompletion);
+        mTileQueryHelper.queryTiles(mHost);
     }
 
     public void hide(int x, int y) {
@@ -259,7 +246,7 @@ public class QSCustomizer extends LinearLayout implements OnMenuItemClickListene
     }
 
     private void save() {
-        if (mFinishedFetchingTiles) {
+        if (mTileQueryHelper.isFinished()) {
             mTileAdapter.saveSpecs(mHost);
         }
     }

diff --git a/packages/SystemUI/src/com/android/systemui/qs/customize/TileQueryHelper.java b/packages/SystemUI/src/com/android/systemui/qs/customize/TileQueryHelper.java
index 8bf4096..9593b0f 100644 (file)
--- a/packages/SystemUI/src/com/android/systemui/qs/customize/TileQueryHelper.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/customize/TileQueryHelper.java
@@ -25,60 +25,63 @@ import android.content.pm.PackageManager;
 import android.content.pm.ResolveInfo;
 import android.graphics.drawable.Drawable;
 import android.os.Handler;
-import android.os.Looper;
 import android.service.quicksettings.TileService;
 import android.text.TextUtils;
+import android.util.ArraySet;
 import android.widget.Button;
 
 import com.android.systemui.Dependency;
 import com.android.systemui.R;
 import com.android.systemui.plugins.qs.QSTile;
 import com.android.systemui.plugins.qs.QSTile.State;
-import com.android.systemui.qs.tileimpl.QSTileImpl.DrawableIcon;
-import com.android.systemui.qs.external.CustomTile;
 import com.android.systemui.qs.QSTileHost;
+import com.android.systemui.qs.external.CustomTile;
+import com.android.systemui.qs.tileimpl.QSTileImpl.DrawableIcon;
 
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
 public class TileQueryHelper {
-
     private static final String TAG = "TileQueryHelper";
 
     private final ArrayList<TileInfo> mTiles = new ArrayList<>();
-    private final ArrayList<String> mSpecs = new ArrayList<>();
+    private final ArraySet<String> mSpecs = new ArraySet<>();
+    private final Handler mBgHandler;
+    private final Handler mMainHandler;
     private final Context mContext;
     private final TileStateListener mListener;
-    private final QSTileHost mHost;
-    private final Runnable mCompletion;
 
-    public TileQueryHelper(Context context, QSTileHost host,
-            TileStateListener listener, Runnable completion) {
+    private boolean mFinished;
+
+    public TileQueryHelper(Context context, TileStateListener listener) {
         mContext = context;
         mListener = listener;
-        mHost = host;
-        mCompletion = completion;
-        addSystemTiles();
-        // TODO: Live?
+        mBgHandler = new Handler(Dependency.get(Dependency.BG_LOOPER));
+        mMainHandler = Dependency.get(Dependency.MAIN_HANDLER);
     }
 
-    private void addSystemTiles() {
+    public void queryTiles(QSTileHost host) {
+        mTiles.clear();
+        mSpecs.clear();
+        mFinished = false;
         // Enqueue jobs to fetch every system tile and then ever package tile.
-        final Handler qsHandler = new Handler((Looper) Dependency.get(Dependency.BG_LOOPER));
-        final Handler mainHandler = new Handler(Looper.getMainLooper());
-        addStockTiles(mainHandler, qsHandler);
-        addPackageTiles(mainHandler, qsHandler);
-        // Then enqueue the completion. It should always be last
-        qsHandler.post(mCompletion);
+        addStockTiles(host);
+        addPackageTiles(host);
+        // TODO: Live?
+    }
+
+    public boolean isFinished() {
+        return mFinished;
     }
 
-    private void addStockTiles(Handler mainHandler, Handler bgHandler) {
+    private void addStockTiles(QSTileHost host) {
         String possible = mContext.getString(R.string.quick_settings_tiles_stock);
         String[] possibleTiles = possible.split(",");
+        final ArrayList<QSTile> tilesToAdd = new ArrayList<>();
         for (int i = 0; i < possibleTiles.length; i++) {
             final String spec = possibleTiles[i];
-            final QSTile tile = mHost.createTile(spec);
+            final QSTile tile = host.createTile(spec);
             if (tile == null) {
                 continue;
             } else if (!tile.isAvailable()) {
@@ -89,28 +92,25 @@ public class TileQueryHelper {
             tile.clearState();
             tile.refreshState();
             tile.setListening(this, false);
-            bgHandler.post(new Runnable() {
-                @Override
-                public void run() {
-                    final QSTile.State state = tile.getState().copy();
-                    // Ignore the current state and get the generic label instead.
-                    state.label = tile.getTileLabel();
-                    tile.destroy();
-                    mainHandler.post(new Runnable() {
-                        @Override
-                        public void run() {
-                            addTile(spec, null, state, true);
-                            mListener.onTilesChanged(mTiles);
-                        }
-                    });
-                }
-            });
+            tile.setTileSpec(spec);
+            tilesToAdd.add(tile);
         }
+
+        mBgHandler.post(() -> {
+            for (QSTile tile : tilesToAdd) {
+                final QSTile.State state = tile.getState().copy();
+                // Ignore the current state and get the generic label instead.
+                state.label = tile.getTileLabel();
+                tile.destroy();
+                addTile(tile.getTileSpec(), null, state, true);
+            }
+            notifyTilesChanged(false);
+        });
     }
 
-    private void addPackageTiles(Handler mainHandler, Handler bgHandler) {
-        bgHandler.post(() -> {
-            Collection<QSTile> params = mHost.getTiles();
+    private void addPackageTiles(final QSTileHost host) {
+        mBgHandler.post(() -> {
+            Collection<QSTile> params = host.getTiles();
             PackageManager pm = mContext.getPackageManager();
             List<ResolveInfo> services = pm.queryIntentServicesAsUser(
                     new Intent(TileService.ACTION_QS_TILE), 0, ActivityManager.getCurrentUser());
@@ -145,9 +145,18 @@ public class TileQueryHelper {
                 icon.mutate();
                 icon.setTint(mContext.getColor(android.R.color.white));
                 CharSequence label = info.serviceInfo.loadLabel(pm);
-                addTile(spec, icon, label != null ? label.toString() : "null", appLabel, mContext);
+                addTile(spec, icon, label != null ? label.toString() : "null", appLabel);
             }
-            mainHandler.post(() -> mListener.onTilesChanged(mTiles));
+
+            notifyTilesChanged(true);
+        });
+    }
+
+    private void notifyTilesChanged(final boolean finished) {
+        final ArrayList<TileInfo> tilesToReturn = new ArrayList<>(mTiles);
+        mMainHandler.post(() -> {
+            mListener.onTilesChanged(tilesToReturn);
+            mFinished = finished;
         });
     }
 
@@ -177,8 +186,8 @@ public class TileQueryHelper {
         mSpecs.add(spec);
     }
 
-    private void addTile(String spec, Drawable drawable, CharSequence label, CharSequence appLabel,
-            Context context) {
+    private void addTile(
+            String spec, Drawable drawable, CharSequence label, CharSequence appLabel) {
         QSTile.State state = new QSTile.State();
         state.label = label;
         state.contentDescription = label;

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 7ad5a59..f5f8ffa 100644 (file)
--- a/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java
@@ -33,7 +33,6 @@ import android.service.quicksettings.TileService;
 import android.support.annotation.VisibleForTesting;
 import android.util.Log;
 
-import com.android.systemui.qs.customize.TileQueryHelper.TileStateListener;
 import com.android.systemui.qs.external.TileLifecycleManager.TileChangeListener;
 
 import java.util.List;

diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileQueryHelperTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileQueryHelperTest.java
index 5ae107a..f63d236 100644 (file)
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileQueryHelperTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/customize/TileQueryHelperTest.java
@@ -15,71 +15,87 @@
 package com.android.systemui.qs.customize;
 
 import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertTrue;
+
+import static org.junit.Assert.assertFalse;
 import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 
-import com.android.systemui.Dependency;
+import android.support.test.filters.SmallTest;
 import android.testing.AndroidTestingRunner;
-import com.android.systemui.SysuiTestCase;
-import com.android.systemui.plugins.qs.QSTile;
-import com.android.systemui.plugins.qs.QSTile.State;
-import com.android.systemui.qs.QSTileHost;
 import android.testing.TestableLooper;
 import android.testing.TestableLooper.RunWithLooper;
 
+import com.android.systemui.Dependency;
+import com.android.systemui.SysuiTestCase;
+import com.android.systemui.qs.QSTileHost;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
 
-import android.os.Message;
-import android.test.suitebuilder.annotation.SmallTest;
+import java.util.ArrayList;
 
 @SmallTest
 @RunWith(AndroidTestingRunner.class)
 @RunWithLooper
 public class TileQueryHelperTest extends SysuiTestCase {
+    @Mock private TileQueryHelper.TileStateListener mListener;
+    @Mock private QSTileHost mQSTileHost;
+
     private TestableLooper mBGLooper;
-    private Runnable mLastCallback;
+
+    private TileQueryHelper mTileQueryHelper;
 
     @Before
     public void setup() {
+        MockitoAnnotations.initMocks(this);
         mBGLooper = TestableLooper.get(this);
         mDependency.injectTestDependency(Dependency.BG_LOOPER, mBGLooper.getLooper());
+        mTileQueryHelper = new TileQueryHelper(mContext, mListener);
+    }
+
+    @Test
+    public void testIsFinished_falseBeforeQuerying() {
+        assertFalse(mTileQueryHelper.isFinished());
+    }
+
+    @Test
+    public void testIsFinished_trueAfterQuerying() {
+        mTileQueryHelper.queryTiles(mQSTileHost);
+
+        mBGLooper.processAllMessages();
+        waitForIdleSync(Dependency.get(Dependency.MAIN_HANDLER));
+
+        assertTrue(mTileQueryHelper.isFinished());
     }
 
     @Test
-    public void testCompletionCalled() {
-        QSTileHost mockHost = mock(QSTileHost.class);
-        TileAdapter mockAdapter = mock(TileAdapter.class);
-        Runnable mockCompletion = mock(Runnable.class);
-        new TileQueryHelper(mContext, mockHost, mockAdapter, mockCompletion);
+    public void testQueryTiles_callsListenerTwice() {
+        mTileQueryHelper.queryTiles(mQSTileHost);
+
         mBGLooper.processAllMessages();
-        verify(mockCompletion).run();
+        waitForIdleSync(Dependency.get(Dependency.MAIN_HANDLER));
+
+        verify(mListener, times(2)).onTilesChanged(any());
     }
 
     @Test
-    public void testCompletionCalledAfterTilesFetched() {
-        QSTile mockTile = mock(QSTile.class);
-        State mockState = mock(State.class);
-        when(mockState.copy()).thenReturn(mockState);
-        when(mockTile.getState()).thenReturn(mockState);
-        when(mockTile.isAvailable()).thenReturn(true);
-
-        QSTileHost mockHost = mock(QSTileHost.class);
-        when(mockHost.createTile(any())).thenReturn(mockTile);
-
-        mBGLooper.setMessageHandler((Message m) -> {
-            mLastCallback = m.getCallback();
-            return true;
-        });
-        TileAdapter mockAdapter = mock(TileAdapter.class);
-        Runnable mockCompletion = mock(Runnable.class);
-        new TileQueryHelper(mContext, mockHost, mockAdapter, mockCompletion);
-
-        // Verify that the last thing in the queue was our callback
+    public void testQueryTiles_isFinishedFalseOnListenerCalls_thenTrueAfterCompletion() {
+        doAnswer(invocation -> {
+            assertFalse(mTileQueryHelper.isFinished());
+            return null;
+        }).when(mListener).onTilesChanged(any());
+
+        mTileQueryHelper.queryTiles(mQSTileHost);
+
         mBGLooper.processAllMessages();
-        assertEquals(mockCompletion, mLastCallback);
+        waitForIdleSync(Dependency.get(Dependency.MAIN_HANDLER));
+
+        assertTrue(mTileQueryHelper.isFinished());
     }
 }