From 4ba3f9aa86b4d96d1c414fe8e6d554c037e2c953 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Fri, 21 Feb 2020 11:02:42 -0500 Subject: [PATCH] Use the correct user context in CustomTile CustomTile was using the SystemUI context (in user 0) to retrieve service information and resources. This failed in 3rd party tiles that could be installed just for a particular user but were not installed in user 0. Additionally, having observed cases where the sysui_qs_tiles setting has "work,work,work,....", this CL cleans that up and prevents it happening in the future by not allowing repeats in the specs list. Fixes some leaks in CustomTile: * destroy CustomTile that changes user * remove Change listerner from TileLifecycleManager Test: atest QSTileHostTest CustomTileTest Test: manual Test: hprof Fixes: 141904434 Change-Id: I92b26294dec63477943d28e2000fe3e0c16f8aa3 Merged-In: I92b26294dec63477943d28e2000fe3e0c16f8aa3 (cherry picked from commit a6862914da91644d89ffa6447818eb192b78a7ab) --- .../com/android/systemui/qs/QSTileHost.java | 44 ++++++++-- .../systemui/qs/external/CustomTile.java | 18 +++-- .../qs/external/TileLifecycleManager.java | 1 + .../systemui/qs/tileimpl/QSFactoryImpl.java | 4 +- .../android/systemui/qs/QSTileHostTest.java | 81 ++++++++++++++++--- .../systemui/qs/external/CustomTileTest.kt | 20 ++++- 6 files changed, 140 insertions(+), 28 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java b/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java index 938f9dbde6ff5..d8ead55a13a8a 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QSTileHost.java @@ -28,6 +28,7 @@ import android.provider.Settings; import android.provider.Settings.Secure; import android.service.quicksettings.Tile; import android.text.TextUtils; +import android.util.ArraySet; import android.util.Log; import com.android.systemui.Dumpable; @@ -61,6 +62,7 @@ import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.function.Predicate; import javax.inject.Inject; @@ -91,6 +93,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D private final ArrayList mQsFactories = new ArrayList<>(); private int mCurrentUser; private final Optional mStatusBarOptional; + private Context mUserContext; @Inject public QSTileHost(Context context, @@ -107,6 +110,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D QSLogger qsLogger) { mIconController = iconController; mContext = context; + mUserContext = context; mTunerService = tunerService; mPluginManager = pluginManager; mDumpManager = dumpManager; @@ -207,6 +211,9 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D return mContext; } + public Context getUserContext() { + return mUserContext; + } public TileServices getTileServices() { return mServices; @@ -227,6 +234,9 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D } final List tileSpecs = loadTileSpecs(mContext, newValue); int currentUser = ActivityManager.getCurrentUser(); + if (currentUser != mCurrentUser) { + mUserContext = mContext.createContextAsUser(UserHandle.of(currentUser), 0); + } if (tileSpecs.equals(mTileSpecs) && currentUser == mCurrentUser) return; mTiles.entrySet().stream().filter(tile -> !tileSpecs.contains(tile.getKey())).forEach( tile -> { @@ -253,6 +263,13 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D mQSLogger.logTileDestroyed(tileSpec, "Tile not available"); } } else { + // This means that the tile is a CustomTile AND the user is different, so let's + // destroy it + if (tile != null) { + tile.destroy(); + Log.d(TAG, "Destroying tile for wrong user: " + tileSpec); + mQSLogger.logTileDestroyed(tileSpec, "Tile for wrong user"); + } Log.d(TAG, "Creating tile: " + tileSpec); try { tile = createTile(tileSpec); @@ -273,7 +290,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D } } mCurrentUser = currentUser; - List currentSpecs = new ArrayList(mTileSpecs); + List currentSpecs = new ArrayList<>(mTileSpecs); mTileSpecs.clear(); mTileSpecs.addAll(tileSpecs); mTiles.clear(); @@ -300,7 +317,7 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D } public void addTile(String spec) { - changeTileSpecs(tileSpecs-> tileSpecs.add(spec)); + changeTileSpecs(tileSpecs-> !tileSpecs.contains(spec) && tileSpecs.add(spec)); } private void changeTileSpecs(Predicate> changeFunction) { @@ -314,9 +331,12 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D } public void addTile(ComponentName tile) { - List newSpecs = new ArrayList<>(mTileSpecs); - newSpecs.add(0, CustomTile.toSpec(tile)); - changeTiles(mTileSpecs, newSpecs); + String spec = CustomTile.toSpec(tile); + if (!mTileSpecs.contains(spec)) { + List newSpecs = new ArrayList<>(mTileSpecs); + newSpecs.add(0, spec); + changeTiles(mTileSpecs, newSpecs); + } } public void removeTile(ComponentName tile) { @@ -380,16 +400,26 @@ public class QSTileHost implements QSHost, Tunable, PluginListener, D } final ArrayList tiles = new ArrayList(); boolean addedDefault = false; + Set addedSpecs = new ArraySet<>(); for (String tile : tileList.split(",")) { tile = tile.trim(); if (tile.isEmpty()) continue; if (tile.equals("default")) { if (!addedDefault) { - tiles.addAll(getDefaultSpecs(context)); + List defaultSpecs = getDefaultSpecs(context); + for (String spec : defaultSpecs) { + if (!addedSpecs.contains(spec)) { + tiles.add(spec); + addedSpecs.add(spec); + } + } addedDefault = true; } } else { - tiles.add(tile); + if (!addedSpecs.contains(tile)) { + tiles.add(tile); + addedSpecs.add(tile); + } } } return tiles; diff --git a/packages/SystemUI/src/com/android/systemui/qs/external/CustomTile.java b/packages/SystemUI/src/com/android/systemui/qs/external/CustomTile.java index 21a424c77a05b..ca2c2d876a390 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/external/CustomTile.java +++ b/packages/SystemUI/src/com/android/systemui/qs/external/CustomTile.java @@ -20,6 +20,7 @@ import static android.view.WindowManager.LayoutParams.TYPE_QS_DIALOG; import android.app.ActivityManager; import android.content.ComponentName; +import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; @@ -72,15 +73,19 @@ public class CustomTile extends QSTileImpl implements TileChangeListener private android.graphics.drawable.Icon mDefaultIcon; private CharSequence mDefaultLabel; + private final Context mUserContext; + private boolean mListening; private boolean mIsTokenGranted; private boolean mIsShowingDialog; - private CustomTile(QSTileHost host, String action) { + private CustomTile(QSTileHost host, String action, Context userContext) { super(host); mWindowManager = WindowManagerGlobal.getWindowManagerService(); mComponent = ComponentName.unflattenFromString(action); mTile = new Tile(); + mUserContext = userContext; + mUser = mUserContext.getUserId(); updateDefaultTileAndIcon(); mServiceManager = host.getTileServices().getTileWrapper(this); if (mServiceManager.isToggleableTile()) { @@ -90,7 +95,6 @@ public class CustomTile extends QSTileImpl implements TileChangeListener mService = mServiceManager.getTileService(); mServiceManager.setTileChangeListener(this); - mUser = ActivityManager.getCurrentUser(); } @Override @@ -100,7 +104,7 @@ public class CustomTile extends QSTileImpl implements TileChangeListener private void updateDefaultTileAndIcon() { try { - PackageManager pm = mContext.getPackageManager(); + PackageManager pm = mUserContext.getPackageManager(); int flags = PackageManager.MATCH_DIRECT_BOOT_UNAWARE | PackageManager.MATCH_DIRECT_BOOT_AWARE; if (isSystemApp(pm)) { flags |= PackageManager.MATCH_DISABLED_COMPONENTS; @@ -318,11 +322,11 @@ public class CustomTile extends QSTileImpl implements TileChangeListener state.state = tileState; Drawable drawable; try { - drawable = mTile.getIcon().loadDrawable(mContext); + drawable = mTile.getIcon().loadDrawable(mUserContext); } catch (Exception e) { Log.w(TAG, "Invalid icon, forcing into unavailable state"); state.state = Tile.STATE_UNAVAILABLE; - drawable = mDefaultIcon.loadDrawable(mContext); + drawable = mDefaultIcon.loadDrawable(mUserContext); } final Drawable drawableF = drawable; @@ -387,7 +391,7 @@ public class CustomTile extends QSTileImpl implements TileChangeListener return ComponentName.unflattenFromString(action); } - public static CustomTile create(QSTileHost host, String spec) { + public static CustomTile create(QSTileHost host, String spec, Context userContext) { if (spec == null || !spec.startsWith(PREFIX) || !spec.endsWith(")")) { throw new IllegalArgumentException("Bad custom tile spec: " + spec); } @@ -395,6 +399,6 @@ public class CustomTile extends QSTileImpl implements TileChangeListener if (action.isEmpty()) { throw new IllegalArgumentException("Empty custom tile spec action"); } - return new CustomTile(host, action); + return new CustomTile(host, action, userContext); } } diff --git a/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java b/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java index 17b0251837e7a..aa51771864b2a 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java +++ b/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java @@ -279,6 +279,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements if (mPackageReceiverRegistered.get() || mUserReceiverRegistered.get()) { stopPackageListening(); } + mChangeListener = null; } private void handleDeath() { diff --git a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSFactoryImpl.java b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSFactoryImpl.java index 554672d880523..1b8717b5049ca 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSFactoryImpl.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSFactoryImpl.java @@ -178,7 +178,9 @@ public class QSFactoryImpl implements QSFactory { } // Custom tiles - if (tileSpec.startsWith(CustomTile.PREFIX)) return CustomTile.create(mHost, tileSpec); + if (tileSpec.startsWith(CustomTile.PREFIX)) { + return CustomTile.create(mHost, tileSpec, mHost.getUserContext()); + } // Debug tiles. if (Build.IS_DEBUGGABLE) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java index 36a9ea6de5aaf..00980129cf741 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSTileHostTest.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.ActivityManager; +import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.os.Handler; @@ -76,7 +77,9 @@ import javax.inject.Provider; public class QSTileHostTest extends SysuiTestCase { private static String MOCK_STATE_STRING = "MockState"; - private static final String CUSTOM_TILE_SPEC = "custom(TEST_PKG/.TEST_CLS)"; + private static ComponentName CUSTOM_TILE = + ComponentName.unflattenFromString("TEST_PKG/.TEST_CLS"); + private static final String CUSTOM_TILE_SPEC = CustomTile.toSpec(CUSTOM_TILE); @Mock private StatusBarIconController mIconController; @@ -114,24 +117,30 @@ public class QSTileHostTest extends SysuiTestCase { mLooper.getLooper(), mPluginManager, mTunerService, mAutoTiles, mDumpManager, mBroadcastDispatcher, mStatusBar, mQSLogger); setUpTileFactory(); + + // Override this config so there are no unexpected tiles + mContext.getOrCreateTestableResources().addOverride( + com.android.internal.R.string.config_defaultExtraQuickSettingsTiles, + ""); + Settings.Secure.putStringForUser(mContext.getContentResolver(), QSTileHost.TILES_SETTING, "", ActivityManager.getCurrentUser()); } private void setUpTileFactory() { when(mMockState.toString()).thenReturn(MOCK_STATE_STRING); + // Only create this kind of tiles when(mDefaultFactory.createTile(anyString())).thenAnswer( invocation -> { String spec = invocation.getArgument(0); - switch (spec) { - case "spec1": - return new TestTile1(mQSTileHost); - case "spec2": - return new TestTile2(mQSTileHost); - case CUSTOM_TILE_SPEC: - return mCustomTile; - default: - return null; + if ("spec1".equals(spec)) { + return new TestTile1(mQSTileHost); + } else if ("spec2".equals(spec)) { + return new TestTile2(mQSTileHost); + } else if (CUSTOM_TILE_SPEC.equals(spec)) { + return mCustomTile; + } else { + return null; } }); when(mCustomTile.isAvailable()).thenReturn(true); @@ -222,6 +231,58 @@ public class QSTileHostTest extends SysuiTestCase { verify(mQSLogger).logTileAdded(CUSTOM_TILE_SPEC); } + @Test + public void testNoRepeatedSpecs_addTile() { + mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, "spec1,spec2"); + + mQSTileHost.addTile("spec1"); + + assertEquals(2, mQSTileHost.mTileSpecs.size()); + assertEquals("spec1", mQSTileHost.mTileSpecs.get(0)); + assertEquals("spec2", mQSTileHost.mTileSpecs.get(1)); + } + + @Test + public void testNoRepeatedSpecs_customTile() { + mQSTileHost.onTuningChanged(QSTileHost.TILES_SETTING, CUSTOM_TILE_SPEC); + + mQSTileHost.addTile(CUSTOM_TILE); + + assertEquals(1, mQSTileHost.mTileSpecs.size()); + assertEquals(CUSTOM_TILE_SPEC, mQSTileHost.mTileSpecs.get(0)); + } + + @Test + public void testLoadTileSpec_repeated() { + List specs = QSTileHost.loadTileSpecs(mContext, "spec1,spec1,spec2"); + + assertEquals(2, specs.size()); + assertEquals("spec1", specs.get(0)); + assertEquals("spec2", specs.get(1)); + } + + @Test + public void testLoadTileSpec_repeatedInDefault() { + mContext.getOrCreateTestableResources() + .addOverride(R.string.quick_settings_tiles_default, "spec1,spec1"); + List specs = QSTileHost.loadTileSpecs(mContext, "default"); + + // Remove spurious tiles, like dbg:mem + specs.removeIf(spec -> !"spec1".equals(spec)); + assertEquals(1, specs.size()); + } + + @Test + public void testLoadTileSpec_repeatedDefaultAndSetting() { + mContext.getOrCreateTestableResources() + .addOverride(R.string.quick_settings_tiles_default, "spec1"); + List specs = QSTileHost.loadTileSpecs(mContext, "default,spec1"); + + // Remove spurious tiles, like dbg:mem + specs.removeIf(spec -> !"spec1".equals(spec)); + assertEquals(1, specs.size()); + } + private static class TestQSTileHost extends QSTileHost { TestQSTileHost(Context context, StatusBarIconController iconController, QSFactoryImpl defaultFactory, Handler mainHandler, Looper bgLooper, diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/external/CustomTileTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/external/CustomTileTest.kt index 9fe2569177d78..608d609492d0b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/external/CustomTileTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/external/CustomTileTest.kt @@ -33,6 +33,7 @@ import com.android.systemui.plugins.qs.QSTile import com.android.systemui.qs.QSTileHost import junit.framework.Assert.assertFalse import junit.framework.Assert.assertTrue +import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -84,13 +85,26 @@ class CustomTileTest : SysuiTestCase() { .thenReturn(mServiceInfo) mServiceInfo.applicationInfo = mApplicationInfo - customTile = CustomTile.create(mTileHost, TILE_SPEC) + customTile = CustomTile.create(mTileHost, TILE_SPEC, mContext) + } + + @Test + fun testCorrectUser() { + assertEquals(0, customTile.user) + + val userContext = mock(Context::class.java) + `when`(userContext.packageManager).thenReturn(mPackageManager) + `when`(userContext.userId).thenReturn(10) + + val tile = CustomTile.create(mTileHost, TILE_SPEC, userContext) + + assertEquals(10, tile.user) } @Test fun testToggleableTileHasBooleanState() { `when`(mTileServiceManager.isToggleableTile).thenReturn(true) - customTile = CustomTile.create(mTileHost, TILE_SPEC) + customTile = CustomTile.create(mTileHost, TILE_SPEC, mContext) assertTrue(customTile.state is QSTile.BooleanState) assertTrue(customTile.newTileState() is QSTile.BooleanState) @@ -105,7 +119,7 @@ class CustomTileTest : SysuiTestCase() { @Test fun testValueUpdatedInBooleanTile() { `when`(mTileServiceManager.isToggleableTile).thenReturn(true) - customTile = CustomTile.create(mTileHost, TILE_SPEC) + customTile = CustomTile.create(mTileHost, TILE_SPEC, mContext) customTile.qsTile.icon = mock(Icon::class.java) `when`(customTile.qsTile.icon.loadDrawable(any(Context::class.java))) .thenReturn(mock(Drawable::class.java))