From aee53bc98dd7f94522e65df1180a8f52ad016976 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Fri, 23 Apr 2021 15:43:26 -0400 Subject: [PATCH] Move preference loading to a background thread To improve the loading time of the Notifications screen. Also introduce placeholder preferences to minimize jank when the data becomes available Test: Robotests, adb shell am start -W -a android.settings.NOTIFICATION_SETTINGS Fixes: 185877371 Change-Id: Id63469fdb42e1a66468f2c5f1da8c366686f06e4 --- .../configure_notification_settings_v2.xml | 9 +++ ...centNotifyingAppsPreferenceController.java | 57 +++++++------------ ...NotifyingAppsPreferenceControllerTest.java | 57 ++++++++++++------- 3 files changed, 67 insertions(+), 56 deletions(-) diff --git a/res/xml/configure_notification_settings_v2.xml b/res/xml/configure_notification_settings_v2.xml index 6b1e69190e5..a0d2dabdc84 100644 --- a/res/xml/configure_notification_settings_v2.xml +++ b/res/xml/configure_notification_settings_v2.xml @@ -42,6 +42,15 @@ android:key="recent_notifications_category" android:title="@string/recent_notifications"> + + + SKIP_SYSTEM_PACKAGES = new ArraySet<>(); private final Fragment mHost; private final PackageManager mPm; @@ -148,13 +149,17 @@ public class RecentNotifyingAppsPreferenceController extends AbstractPreferenceC @VisibleForTesting void refreshUi(Context prefContext) { - reloadData(); - final List recentApps = getDisplayableRecentAppList(); - if (recentApps != null && !recentApps.isEmpty()) { - displayRecentApps(prefContext, recentApps); - } else { - displayOnlyAllAppsLink(); - } + ThreadUtils.postOnBackgroundThread(() -> { + reloadData(); + final List recentApps = getDisplayableRecentAppList(); + ThreadUtils.postOnMainThread(() -> { + if (recentApps != null && !recentApps.isEmpty()) { + displayRecentApps(prefContext, recentApps); + } else { + displayOnlyAllAppsLink(); + } + }); + }); } @VisibleForTesting @@ -198,8 +203,7 @@ public class RecentNotifyingAppsPreferenceController extends AbstractPreferenceC } } - @VisibleForTesting - static String getKey(int userId, String pkg) { + private static String getKey(int userId, String pkg) { return userId + "|" + pkg; } @@ -221,19 +225,9 @@ public class RecentNotifyingAppsPreferenceController extends AbstractPreferenceC mSeeAllPref.setSummary(null); mSeeAllPref.setIcon(R.drawable.ic_chevron_right_24dp); - // Rebind prefs/avoid adding new prefs if possible. Adding/removing prefs causes jank. - // Build a cached preference pool - final Map appPreferences = new ArrayMap<>(); - int prefCount = mCategory.getPreferenceCount(); - for (int i = 0; i < prefCount; i++) { - final Preference pref = mCategory.getPreference(i); - final String key = pref.getKey(); - if (!TextUtils.equals(key, KEY_SEE_ALL)) { - appPreferences.put(key, (PrimarySwitchPreference) pref); - } - } + int keyIndex = 1; final int recentAppsCount = recentApps.size(); - for (int i = 0; i < recentAppsCount; i++) { + for (int i = 0; i < recentAppsCount; i++, keyIndex++) { final NotifyingApp app = recentApps.get(i); // Bind recent apps to existing prefs if possible, or create a new pref. final String pkgName = app.getPackage(); @@ -243,20 +237,12 @@ public class RecentNotifyingAppsPreferenceController extends AbstractPreferenceC continue; } - boolean rebindPref = true; - PrimarySwitchPreference pref = appPreferences.remove(getKey(app.getUserId(), - pkgName)); - if (pref == null) { - pref = new PrimarySwitchPreference(prefContext); - rebindPref = false; - } - pref.setKey(getKey(app.getUserId(), pkgName)); + PrimarySwitchPreference pref = mCategory.findPreference(KEY_PLACEHOLDER + keyIndex); pref.setTitle(appEntry.label); pref.setIcon(mIconDrawableFactory.getBadgedIcon(appEntry.info)); pref.setIconSize(TwoTargetPreference.ICON_SIZE_SMALL); pref.setSummary(StringUtil.formatRelativeTime(mContext, System.currentTimeMillis() - app.getLastNotified(), true)); - pref.setOrder(i); Bundle args = new Bundle(); args.putString(AppInfoBase.ARG_PACKAGE_NAME, pkgName); args.putInt(AppInfoBase.ARG_PACKAGE_UID, appEntry.info.uid); @@ -280,13 +266,10 @@ public class RecentNotifyingAppsPreferenceController extends AbstractPreferenceC pref.setChecked( !mNotificationBackend.getNotificationsBanned(pkgName, appEntry.info.uid)); - if (!rebindPref) { - mCategory.addPreference(pref); - } } - // Remove unused prefs from pref cache pool - for (Preference unusedPrefs : appPreferences.values()) { - mCategory.removePreference(unusedPrefs); + // If there are less than SHOW_RECENT_APP_COUNT recent apps, remove placeholders + for (int i = keyIndex; i <= SHOW_RECENT_APP_COUNT; i++) { + mCategory.removePreferenceRecursively(KEY_PLACEHOLDER + i); } } diff --git a/tests/robotests/src/com/android/settings/notification/RecentNotifyingAppsPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/notification/RecentNotifyingAppsPreferenceControllerTest.java index 6226b9a318b..983bf535bdd 100644 --- a/tests/robotests/src/com/android/settings/notification/RecentNotifyingAppsPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/notification/RecentNotifyingAppsPreferenceControllerTest.java @@ -48,6 +48,7 @@ import android.service.notification.NotifyingApp; import android.text.TextUtils; import com.android.settings.R; +import com.android.settings.widget.PrimarySwitchPreference; import com.android.settingslib.applications.AppUtils; import com.android.settingslib.applications.ApplicationsState; import com.android.settingslib.applications.instantapps.InstantAppDataProvider; @@ -80,6 +81,9 @@ public class RecentNotifyingAppsPreferenceControllerTest { private PreferenceScreen mScreen; @Mock private PreferenceCategory mCategory; + private PrimarySwitchPreference mApp1; + private PrimarySwitchPreference mApp2; + private PrimarySwitchPreference mApp3; @Mock private Preference mSeeAllPref; @Mock @@ -115,6 +119,15 @@ public class RecentNotifyingAppsPreferenceControllerTest { mController = new RecentNotifyingAppsPreferenceController( mContext, mBackend, mIUsageStatsManager, mUserManager, mAppState, mHost); when(mScreen.findPreference(anyString())).thenReturn(mCategory); + mApp1 = new PrimarySwitchPreference(mContext); + mApp1.setKey("app1"); + mApp2 = new PrimarySwitchPreference(mContext); + mApp2.setKey("app2"); + mApp3 = new PrimarySwitchPreference(mContext); + mApp3.setKey("app3"); + when(mCategory.findPreference("app1")).thenReturn(mApp1); + when(mCategory.findPreference("app2")).thenReturn(mApp2); + when(mCategory.findPreference("app3")).thenReturn(mApp3); when(mScreen.findPreference(RecentNotifyingAppsPreferenceController.KEY_SEE_ALL)) .thenReturn(mSeeAllPref); @@ -169,12 +182,18 @@ public class RecentNotifyingAppsPreferenceControllerTest { app2.mPackage = "pkg.class2"; app2.mTimeStamp = System.currentTimeMillis() - 1000; events.add(app2); + ApplicationsState.AppEntry app1Entry = mock(ApplicationsState.AppEntry.class); + ApplicationsState.AppEntry app2Entry = mock(ApplicationsState.AppEntry.class); + app1Entry.info = mApplicationInfo; + app1Entry.label = "app 1"; + app2Entry.info = mApplicationInfo; + app2Entry.label = "app 2"; // app1, app2 are valid apps. app3 is invalid. when(mAppState.getEntry(app.getPackageName(), UserHandle.myUserId())) - .thenReturn(mAppEntry); + .thenReturn(app1Entry); when(mAppState.getEntry(app1.getPackageName(), UserHandle.myUserId())) - .thenReturn(mAppEntry); + .thenReturn(app2Entry); when(mAppState.getEntry(app2.getPackageName(), UserHandle.myUserId())) .thenReturn(null); when(mPackageManager.resolveActivity(any(Intent.class), anyInt())).thenReturn( @@ -192,7 +211,10 @@ public class RecentNotifyingAppsPreferenceControllerTest { verify(mCategory).setTitle(R.string.recent_notifications); // Only add app1 & app2. app3 skipped because it's invalid app. - verify(mCategory, times(2)).addPreference(any(Preference.class)); + assertThat(mApp1.getTitle()).isEqualTo(app1Entry.label); + assertThat(mApp2.getTitle()).isEqualTo(app2Entry.label); + + verify(mCategory).removePreferenceRecursively(mApp3.getKey()); verify(mSeeAllPref).setSummary(null); verify(mSeeAllPref).setIcon(R.drawable.ic_chevron_right_24dp); @@ -209,22 +231,24 @@ public class RecentNotifyingAppsPreferenceControllerTest { Event app1 = new Event(); app1.mEventType = Event.NOTIFICATION_INTERRUPTION; app1.mPackage = "com.foo.barinstant"; - app1.mTimeStamp = System.currentTimeMillis() + 200; + app1.mTimeStamp = System.currentTimeMillis() - 200; events.add(app1); UsageEvents usageEvents = getUsageEvents( new String[] {"com.foo.bar", "com.foo.barinstant"}, events); when(mIUsageStatsManager.queryEventsForUser(anyLong(), anyLong(), anyInt(), anyString())) .thenReturn(usageEvents); + ApplicationsState.AppEntry appEntry = mock(ApplicationsState.AppEntry.class); ApplicationsState.AppEntry app1Entry = mock(ApplicationsState.AppEntry.class); - ApplicationsState.AppEntry app2Entry = mock(ApplicationsState.AppEntry.class); + appEntry.info = mApplicationInfo; + appEntry.label = "app 1"; app1Entry.info = mApplicationInfo; - app2Entry.info = mApplicationInfo; + app1Entry.label = "app 2"; when(mAppState.getEntry( - app.getPackageName(), UserHandle.myUserId())).thenReturn(app1Entry); + app.getPackageName(), UserHandle.myUserId())).thenReturn(appEntry); when(mAppState.getEntry( - app1.getPackageName(), UserHandle.myUserId())).thenReturn(app2Entry); + app1.getPackageName(), UserHandle.myUserId())).thenReturn(app1Entry); // Only the regular app app1 should have its intent resolve. when(mPackageManager.resolveActivity(argThat(intentMatcher(app.getPackageName())), @@ -233,7 +257,7 @@ public class RecentNotifyingAppsPreferenceControllerTest { // Make sure app2 is considered an instant app. ReflectionHelpers.setStaticField(AppUtils.class, "sInstantAppDataProvider", (InstantAppDataProvider) (ApplicationInfo info) -> { - if (info == app2Entry.info) { + if (info == app1Entry.info) { return true; } else { return false; @@ -242,15 +266,10 @@ public class RecentNotifyingAppsPreferenceControllerTest { mController.displayPreference(mScreen); - ArgumentCaptor prefCaptor = ArgumentCaptor.forClass(Preference.class); - verify(mCategory, times(2)).addPreference(prefCaptor.capture()); - List prefs = prefCaptor.getAllValues(); - assertThat(prefs.get(1).getKey()).isEqualTo( - RecentNotifyingAppsPreferenceController.getKey(UserHandle.myUserId(), - app.getPackageName())); - assertThat(prefs.get(0).getKey()).isEqualTo( - RecentNotifyingAppsPreferenceController.getKey(UserHandle.myUserId(), - app1.getPackageName())); + assertThat(mApp1.getTitle()).isEqualTo(appEntry.label); + assertThat(mApp2.getTitle()).isEqualTo(app1Entry.label); + + verify(mCategory).removePreferenceRecursively(mApp3.getKey()); } @Test @@ -274,7 +293,7 @@ public class RecentNotifyingAppsPreferenceControllerTest { mController.displayPreference(mScreen); - verify(mCategory).addPreference(argThat(summaryMatches("Just now"))); + assertThat(mApp1.getSummary()).isEqualTo("Just now"); } @Test