From 86419864012ad13278242f446f8eea27eb35271d Mon Sep 17 00:00:00 2001 From: Raff Tsai Date: Tue, 3 Sep 2019 10:00:56 +0800 Subject: [PATCH] Prevent UI jank Move hide preference logic to getAvailabilityStatus, it can remove the preference before onresume. Fixes: 140366463 Test: manual, robolectric Change-Id: Ie11b5357b1e9340b30b8f19eac60c479cdb7687e --- .../RestrictAppPreferenceController.java | 16 ++--- .../fuelgauge/SmartBatterySettings.java | 1 - .../RestrictAppPreferenceControllerTest.java | 72 ++++++++++++------- 3 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/com/android/settings/fuelgauge/RestrictAppPreferenceController.java b/src/com/android/settings/fuelgauge/RestrictAppPreferenceController.java index d3c1d54e1af..4dd44195830 100644 --- a/src/com/android/settings/fuelgauge/RestrictAppPreferenceController.java +++ b/src/com/android/settings/fuelgauge/RestrictAppPreferenceController.java @@ -49,6 +49,7 @@ public class RestrictAppPreferenceController extends BasePreferenceController { super(context, KEY_RESTRICT_APP); mAppOpsManager = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE); mUserManager = context.getSystemService(UserManager.class); + mAppInfos = BatteryTipUtils.getRestrictedAppsList(mAppOpsManager, mUserManager); } public RestrictAppPreferenceController(InstrumentedPreferenceFragment preferenceFragment) { @@ -58,21 +59,14 @@ public class RestrictAppPreferenceController extends BasePreferenceController { @Override public int getAvailabilityStatus() { - return AVAILABLE; + return mAppInfos.size() > 0 ? AVAILABLE : CONDITIONALLY_UNAVAILABLE; } @Override - public void updateState(Preference preference) { - super.updateState(preference); - - mAppInfos = BatteryTipUtils.getRestrictedAppsList(mAppOpsManager, mUserManager); - + public CharSequence getSummary() { final int num = mAppInfos.size(); - // Don't show it if no app been restricted - preference.setVisible(num > 0); - preference.setSummary( - mContext.getResources().getQuantityString(R.plurals.restricted_app_summary, num, - num)); + return mContext.getResources().getQuantityString(R.plurals.restricted_app_summary, num, + num); } @Override diff --git a/src/com/android/settings/fuelgauge/SmartBatterySettings.java b/src/com/android/settings/fuelgauge/SmartBatterySettings.java index 8fc8f87bcb4..130c1f2ad6f 100644 --- a/src/com/android/settings/fuelgauge/SmartBatterySettings.java +++ b/src/com/android/settings/fuelgauge/SmartBatterySettings.java @@ -68,7 +68,6 @@ public class SmartBatterySettings extends DashboardFragment { Context context, SettingsActivity settingsActivity, InstrumentedPreferenceFragment fragment) { final List controllers = new ArrayList<>(); - controllers.add(new SmartBatteryPreferenceController(context)); if (settingsActivity != null && fragment != null) { controllers.add( new RestrictAppPreferenceController(fragment)); diff --git a/tests/robotests/src/com/android/settings/fuelgauge/RestrictAppPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/RestrictAppPreferenceControllerTest.java index 78687688df4..774fba22885 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/RestrictAppPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/RestrictAppPreferenceControllerTest.java @@ -23,8 +23,10 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.app.Activity; import android.app.AppOpsManager; @@ -34,6 +36,8 @@ import android.os.UserHandle; import android.os.UserManager; import androidx.preference.Preference; +import androidx.preference.PreferenceManager; +import androidx.preference.PreferenceScreen; import com.android.settings.R; import com.android.settings.core.InstrumentedPreferenceFragment; @@ -46,10 +50,10 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.Robolectric; +import org.robolectric.RobolectricTestRunner; import java.util.ArrayList; import java.util.List; -import org.robolectric.RobolectricTestRunner; @RunWith(RobolectricTestRunner.class) public class RestrictAppPreferenceControllerTest { @@ -70,25 +74,23 @@ public class RestrictAppPreferenceControllerTest { private AppOpsManager.PackageOps mAllowedPackageOps; private AppOpsManager.PackageOps mOtherUserPackageOps; private List mPackageOpsList; - private RestrictAppPreferenceController mRestrictAppPreferenceController; private Preference mPreference; + private PreferenceScreen mPreferenceScreen; private Context mContext; @Before public void setUp() { MockitoAnnotations.initMocks(this); - final AppOpsManager.OpEntry allowOpEntry = new AppOpsManager.OpEntry( - AppOpsManager.OP_RUN_ANY_IN_BACKGROUND, false, AppOpsManager.MODE_ALLOWED, - null /*accessTimes*/, null /*rejectTimes*/, null /*durations*/, - null /* proxyUids */, null /* proxyPackages */); final List allowOps = new ArrayList<>(); - allowOps.add(allowOpEntry); - final AppOpsManager.OpEntry restrictedOpEntry = new AppOpsManager.OpEntry( - AppOpsManager.OP_RUN_ANY_IN_BACKGROUND, false, AppOpsManager.MODE_IGNORED, - null /*accessTimes*/, null /*rejectTimes*/, null /*durations*/, - null /* proxyUids */, null /* proxyPackages */); + allowOps.add(new AppOpsManager.OpEntry( + AppOpsManager.OP_RUN_ANY_IN_BACKGROUND, false, AppOpsManager.MODE_ALLOWED, + null /*accessTimes*/, null /*rejectTimes*/, null /*durations*/, + null /* proxyUids */, null /* proxyPackages */)); final List restrictedOps = new ArrayList<>(); - restrictedOps.add(restrictedOpEntry); + restrictedOps.add(new AppOpsManager.OpEntry( + AppOpsManager.OP_RUN_ANY_IN_BACKGROUND, false, AppOpsManager.MODE_IGNORED, + null /*accessTimes*/, null /*rejectTimes*/, null /*durations*/, + null /* proxyUids */, null /* proxyPackages */)); mAllowedPackageOps = new AppOpsManager.PackageOps( ALLOWED_PACKAGE_NAME, ALLOWED_UID, allowOps); mRestrictedPackageOps = new AppOpsManager.PackageOps( @@ -100,11 +102,15 @@ public class RestrictAppPreferenceControllerTest { doReturn(mAppOpsManager).when(mContext).getSystemService(Context.APP_OPS_SERVICE); doReturn(mUserManager).when(mContext).getSystemService(UserManager.class); doReturn(mContext).when(mFragment).getContext(); - mRestrictAppPreferenceController = - new RestrictAppPreferenceController(mFragment); + mPackageOpsList = new ArrayList<>(); mPreference = new Preference(mContext); - mPreference.setKey(mRestrictAppPreferenceController.getPreferenceKey()); + mPreference.setKey(RestrictAppPreferenceController.KEY_RESTRICT_APP); + mPreferenceScreen = spy(new PreferenceScreen(mContext, null)); + when(mPreferenceScreen.getPreferenceManager()).thenReturn(mock(PreferenceManager.class)); + when(mPreferenceScreen.getContext()).thenReturn(mContext); + when(mPreferenceScreen.findPreference( + RestrictAppPreferenceController.KEY_RESTRICT_APP)).thenReturn(mPreference); final List userHandles = new ArrayList<>(); userHandles.add(new UserHandle(0)); @@ -112,40 +118,49 @@ public class RestrictAppPreferenceControllerTest { } @Test - public void testUpdateState_oneApp_showCorrectSummary() { + public void updateState_oneApp_showCorrectSummary() { mPackageOpsList.add(mRestrictedPackageOps); doReturn(mPackageOpsList).when(mAppOpsManager).getPackagesForOps(any(int[].class)); - mRestrictAppPreferenceController.updateState(mPreference); + final RestrictAppPreferenceController controller = new RestrictAppPreferenceController( + mFragment); + controller.displayPreference(mPreferenceScreen); + controller.updateState(mPreference); assertThat(mPreference.getSummary()).isEqualTo("Limiting battery usage for 1 app"); } @Test - public void testUpdateState_twoRestrictedAppsForPrimaryUser_visibleAndShowCorrectSummary() { + public void updateState_twoRestrictedAppsForPrimaryUser_visibleAndShowCorrectSummary() { mPackageOpsList.add(mRestrictedPackageOps); mPackageOpsList.add(mRestrictedPackageOps); mPackageOpsList.add(mAllowedPackageOps); mPackageOpsList.add(mOtherUserPackageOps); doReturn(mPackageOpsList).when(mAppOpsManager).getPackagesForOps(any(int[].class)); - mRestrictAppPreferenceController.updateState(mPreference); + final RestrictAppPreferenceController controller = new RestrictAppPreferenceController( + mFragment); + controller.displayPreference(mPreferenceScreen); + controller.updateState(mPreference); assertThat(mPreference.getSummary()).isEqualTo("Limiting battery usage for 2 apps"); assertThat(mPreference.isVisible()).isTrue(); } @Test - public void testUpdateState_oneRestrictedAppForTwoUsers_showSummaryAndContainCorrectApp() { + public void updateState_oneRestrictedAppForTwoUsers_showSummaryAndContainCorrectApp() { // Two packageOps share same package name but different uid. mPackageOpsList.add(mRestrictedPackageOps); mPackageOpsList.add(mOtherUserPackageOps); doReturn(mPackageOpsList).when(mAppOpsManager).getPackagesForOps(any(int[].class)); - mRestrictAppPreferenceController.updateState(mPreference); + final RestrictAppPreferenceController controller = new RestrictAppPreferenceController( + mFragment); + controller.displayPreference(mPreferenceScreen); + controller.updateState(mPreference); assertThat(mPreference.getSummary()).isEqualTo("Limiting battery usage for 1 app"); - assertThat(mRestrictAppPreferenceController.mAppInfos).containsExactly( + assertThat(controller.mAppInfos).containsExactly( new AppInfo.Builder() .setUid(RESTRICTED_UID) .setPackageName(RESTRICTED_PACKAGE_NAME) @@ -153,20 +168,25 @@ public class RestrictAppPreferenceControllerTest { } @Test - public void testUpdateState_zeroRestrictApp_inVisible() { + public void updateState_zeroRestrictApp_inVisible() { mPackageOpsList.add(mAllowedPackageOps); doReturn(mPackageOpsList).when(mAppOpsManager).getPackagesForOps(any(int[].class)); - mRestrictAppPreferenceController.updateState(mPreference); + final RestrictAppPreferenceController controller = new RestrictAppPreferenceController( + mFragment); + controller.displayPreference(mPreferenceScreen); + controller.updateState(mPreference); assertThat(mPreference.isVisible()).isFalse(); } @Test - public void testHandlePreferenceTreeClick_startFragment() { + public void handlePreferenceTreeClick_startFragment() { final ArgumentCaptor intent = ArgumentCaptor.forClass(Intent.class); - mRestrictAppPreferenceController.handlePreferenceTreeClick(mPreference); + final RestrictAppPreferenceController controller = new RestrictAppPreferenceController( + mFragment); + controller.handlePreferenceTreeClick(mPreference); verify(mContext).startActivity(intent.capture()); assertThat(intent.getValue().getStringExtra(EXTRA_SHOW_FRAGMENT))