From e4b2b77452ff5dbbcea0fa650da8606cde6328e9 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 Merged-In: I77745b60f84510554bff1870a5bb7a8013eab528 (cherry picked from commit 20df25e6b91fac0c10ce358210fbd37d7bdb754a) --- .../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 57697a689e5..199034c74c7 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.updateCategoryFromBlacklist(sTileBlacklist); - 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 525b6f83aa3..b66de9dc56c 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; @@ -88,6 +89,7 @@ public class CategoryManager { public synchronized void updateCategoryFromBlacklist(Set tileBlacklist) { if (mCategories == null) { Log.w(TAG, "Category is null, skipping blacklist update"); + return; } for (int i = 0; i < mCategories.size(); i++) { DashboardCategory category = mCategories.get(i); @@ -100,6 +102,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. @@ -108,6 +135,7 @@ public class CategoryManager { private synchronized void tryInitCategories(Context context, boolean forceClearCache) { if (mCategories == null) { + final boolean firstLoading = mCategoryByKeyMap.isEmpty(); if (forceClearCache) { mTileByComponentCache.clear(); } @@ -119,6 +147,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 8084038ba68..69f1f1b0065 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