From 20df25e6b91fac0c10ce358210fbd37d7bdb754a Mon Sep 17 00:00:00 2001 From: Jason Chiu Date: Wed, 30 Sep 2020 14:08:33 +0800 Subject: [PATCH] Reduce the flickering of injected items when package is changed Root cause: Settings listens to four package-related broadcasts in order to refresh injected items because UI data may change. However, when the system is updating apps on the first boot, it triggers a burst of broadcasts. For each broadcast Settings will reload and then redraw all injected items, which leads to the flickering. Solution: 1. When Settings recieves a broadcast, check if there are already two reloading tasks to avoid redundant updates. 2. In the reloading task, check if any injected item is changed, added, or removed to notify categories changed. 3. Only refresh the UI when any of the changed items belongs to the current page. Bug: 166785977 Bug: 168309941 Test: manual, robotest Change-Id: I77745b60f84510554bff1870a5bb7a8013eab528 --- .../settings/core/SettingsBaseActivity.java | 96 ++++++++++++++++--- .../settings/dashboard/CategoryManager.java | 31 ++++++ .../settings/dashboard/DashboardFragment.java | 19 +++- 3 files changed, 126 insertions(+), 20 deletions(-) diff --git a/src/com/android/settings/core/SettingsBaseActivity.java b/src/com/android/settings/core/SettingsBaseActivity.java index 68fa1d3e4aa..d160dc5e9d5 100644 --- a/src/com/android/settings/core/SettingsBaseActivity.java +++ b/src/com/android/settings/core/SettingsBaseActivity.java @@ -41,11 +41,14 @@ import androidx.fragment.app.FragmentActivity; import com.android.settings.R; import com.android.settings.SubSettings; import com.android.settings.dashboard.CategoryManager; +import com.android.settingslib.drawer.Tile; import com.google.android.setupcompat.util.WizardManagerHelper; import java.util.ArrayList; import java.util.List; +import java.util.Map; +import java.util.Set; public class SettingsBaseActivity extends FragmentActivity { @@ -59,6 +62,7 @@ public class SettingsBaseActivity extends FragmentActivity { private final PackageReceiver mPackageReceiver = new PackageReceiver(); private final List mCategoryListeners = new ArrayList<>(); + private int mCategoriesUpdateTaskCount; @Override protected void onCreate(@Nullable Bundle savedInstanceState) { @@ -147,10 +151,10 @@ public class SettingsBaseActivity extends FragmentActivity { ((ViewGroup) findViewById(R.id.content_frame)).addView(view, params); } - private void onCategoriesChanged() { + private void onCategoriesChanged(Set categories) { final int N = mCategoryListeners.size(); for (int i = 0; i < N; i++) { - mCategoryListeners.get(i).onCategoriesChanged(); + mCategoryListeners.get(i).onCategoriesChanged(categories); } } @@ -194,38 +198,100 @@ public class SettingsBaseActivity extends FragmentActivity { * Updates dashboard categories. Only necessary to call this after setTileEnabled */ public void updateCategories() { - new CategoriesUpdateTask().execute(); + updateCategories(false /* fromBroadcast */); + } + + private void updateCategories(boolean fromBroadcast) { + // Only allow at most 2 tasks existing at the same time since when the first one is + // executing, there may be new data from the second update request. + // Ignore the third update request because the second task is still waiting for the first + // task to complete in a serial thread, which will get the latest data. + if (mCategoriesUpdateTaskCount < 2) { + new CategoriesUpdateTask().execute(fromBroadcast); + } } public interface CategoryListener { - void onCategoriesChanged(); + /** + * @param categories the changed categories that have to be refreshed, or null to force + * refreshing all. + */ + void onCategoriesChanged(@Nullable Set categories); } - private class CategoriesUpdateTask extends AsyncTask { + private class CategoriesUpdateTask extends AsyncTask> { + private final Context mContext; private final CategoryManager mCategoryManager; + private Map mPreviousTileMap; public CategoriesUpdateTask() { - mCategoryManager = CategoryManager.get(SettingsBaseActivity.this); + mCategoriesUpdateTaskCount++; + mContext = SettingsBaseActivity.this; + mCategoryManager = CategoryManager.get(mContext); } @Override - protected Void doInBackground(Void... params) { - mCategoryManager.reloadAllCategories(SettingsBaseActivity.this); - return null; - } - - @Override - protected void onPostExecute(Void result) { + protected Set doInBackground(Boolean... params) { + mPreviousTileMap = mCategoryManager.getTileByComponentMap(); + mCategoryManager.reloadAllCategories(mContext); mCategoryManager.updateCategoryFromDenylist(sTileDenylist); - onCategoriesChanged(); + return getChangedCategories(params[0]); + } + + @Override + protected void onPostExecute(Set categories) { + if (categories == null || !categories.isEmpty()) { + onCategoriesChanged(categories); + } + mCategoriesUpdateTaskCount--; + } + + // Return the changed categories that have to be refreshed, or null to force refreshing all. + private Set getChangedCategories(boolean fromBroadcast) { + if (!fromBroadcast) { + // Always refresh for non-broadcast case. + return null; + } + + final Set changedCategories = new ArraySet<>(); + final Map currentTileMap = + mCategoryManager.getTileByComponentMap(); + currentTileMap.forEach((component, currentTile) -> { + final Tile previousTile = mPreviousTileMap.get(component); + // Check if the tile is newly added. + if (previousTile == null) { + Log.i(TAG, "Tile added: " + component.flattenToShortString()); + changedCategories.add(currentTile.getCategory()); + return; + } + + // Check if the title or summary has changed. + if (!TextUtils.equals(currentTile.getTitle(mContext), + previousTile.getTitle(mContext)) + || !TextUtils.equals(currentTile.getSummary(mContext), + previousTile.getSummary(mContext))) { + Log.i(TAG, "Tile changed: " + component.flattenToShortString()); + changedCategories.add(currentTile.getCategory()); + } + }); + + // Check if any previous tile is removed. + final Set removal = new ArraySet(mPreviousTileMap.keySet()); + removal.removeAll(currentTileMap.keySet()); + removal.forEach(component -> { + Log.i(TAG, "Tile removed: " + component.flattenToShortString()); + changedCategories.add(mPreviousTileMap.get(component).getCategory()); + }); + + return changedCategories; } } private class PackageReceiver extends BroadcastReceiver { @Override public void onReceive(Context context, Intent intent) { - updateCategories(); + updateCategories(true /* fromBroadcast */); } } } diff --git a/src/com/android/settings/dashboard/CategoryManager.java b/src/com/android/settings/dashboard/CategoryManager.java index 51c485ae74e..2a82abe4859 100644 --- a/src/com/android/settings/dashboard/CategoryManager.java +++ b/src/com/android/settings/dashboard/CategoryManager.java @@ -41,6 +41,7 @@ import java.util.Set; public class CategoryManager { private static final String TAG = "CategoryManager"; + private static final boolean DEBUG = false; private static CategoryManager sInstance; private final InterestingConfigChanges mInterestingConfigChanges; @@ -92,6 +93,7 @@ public class CategoryManager { public synchronized void updateCategoryFromDenylist(Set tileDenylist) { if (mCategories == null) { Log.w(TAG, "Category is null, skipping denylist update"); + return; } for (int i = 0; i < mCategories.size(); i++) { DashboardCategory category = mCategories.get(i); @@ -104,6 +106,31 @@ public class CategoryManager { } } + /** Return the current tile map */ + public synchronized Map getTileByComponentMap() { + final Map result = new ArrayMap<>(); + if (mCategories == null) { + Log.w(TAG, "Category is null, no tiles"); + return result; + } + mCategories.forEach(category -> { + for (int i = 0; i < category.getTilesCount(); i++) { + final Tile tile = category.getTile(i); + result.put(tile.getIntent().getComponent(), tile); + } + }); + return result; + } + + private void logTiles(Context context) { + if (DEBUG) { + getTileByComponentMap().forEach((component, tile) -> { + Log.d(TAG, "Tile: " + tile.getCategory().replace("com.android.settings.", "") + + ": " + tile.getTitle(context) + ", " + component.flattenToShortString()); + }); + } + } + private synchronized void tryInitCategories(Context context) { // Keep cached tiles by default. The cache is only invalidated when InterestingConfigChange // happens. @@ -112,6 +139,7 @@ public class CategoryManager { private synchronized void tryInitCategories(Context context, boolean forceClearCache) { if (mCategories == null) { + final boolean firstLoading = mCategoryByKeyMap.isEmpty(); if (forceClearCache) { mTileByComponentCache.clear(); } @@ -123,6 +151,9 @@ public class CategoryManager { backwardCompatCleanupForCategory(mTileByComponentCache, mCategoryByKeyMap); sortCategories(context, mCategoryByKeyMap); filterDuplicateTiles(mCategoryByKeyMap); + if (firstLoading) { + logTiles(context); + } } } diff --git a/src/com/android/settings/dashboard/DashboardFragment.java b/src/com/android/settings/dashboard/DashboardFragment.java index a273d6c1794..e0c9820042a 100644 --- a/src/com/android/settings/dashboard/DashboardFragment.java +++ b/src/com/android/settings/dashboard/DashboardFragment.java @@ -56,6 +56,7 @@ import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.concurrent.ExecutionException; /** @@ -160,13 +161,21 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment } @Override - public void onCategoriesChanged() { - final DashboardCategory category = - mDashboardFeatureProvider.getTilesForCategory(getCategoryKey()); - if (category == null) { + public void onCategoriesChanged(Set categories) { + final String categoryKey = getCategoryKey(); + final DashboardCategory dashboardCategory = + mDashboardFeatureProvider.getTilesForCategory(categoryKey); + if (dashboardCategory == null) { return; } - refreshDashboardTiles(getLogTag()); + + if (categories == null) { + // force refreshing + refreshDashboardTiles(getLogTag()); + } else if (categories.contains(categoryKey)) { + Log.i(TAG, "refresh tiles for " + categoryKey); + refreshDashboardTiles(getLogTag()); + } } @Override