From 67d149ae377bd943eea3c136ebabd921c465afb7 Mon Sep 17 00:00:00 2001 From: Narayan Kamath Date: Wed, 15 May 2019 18:19:44 +0100 Subject: [PATCH 1/2] Hide AppButtonsPreferenceController for system modules. Bug: 131927465 Test: atest AppButtonsPreferenceControllerTest Change-Id: I2d3aa3429f61325afe49bfe322522fe9ccd03b2c --- .../AppButtonsPreferenceController.java | 11 +++- .../AppButtonsPreferenceControllerTest.java | 53 ++++++++++++++++--- 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/com/android/settings/applications/appinfo/AppButtonsPreferenceController.java b/src/com/android/settings/applications/appinfo/AppButtonsPreferenceController.java index 7339f2132b2..e15b0e3e4b1 100644 --- a/src/com/android/settings/applications/appinfo/AppButtonsPreferenceController.java +++ b/src/com/android/settings/applications/appinfo/AppButtonsPreferenceController.java @@ -160,8 +160,7 @@ public class AppButtonsPreferenceController extends BasePreferenceController imp @Override public int getAvailabilityStatus() { // TODO(b/37313605): Re-enable once this controller supports instant apps - return mAppEntry != null && !AppUtils.isInstant(mAppEntry.info) - ? AVAILABLE : DISABLED_FOR_USER; + return isInstantApp() || isSystemModule() ? DISABLED_FOR_USER : AVAILABLE; } @Override @@ -685,6 +684,14 @@ public class AppButtonsPreferenceController extends BasePreferenceController imp } } + private boolean isInstantApp() { + return mAppEntry != null && AppUtils.isInstant(mAppEntry.info); + } + + private boolean isSystemModule() { + return mAppEntry != null && AppUtils.isSystemModule(mContext, mAppEntry.info.packageName); + } + /** * Changes the status of disable/enable for a package */ diff --git a/tests/robotests/src/com/android/settings/applications/appinfo/AppButtonsPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/applications/appinfo/AppButtonsPreferenceControllerTest.java index ff33d26ea21..e42c3d21f74 100644 --- a/tests/robotests/src/com/android/settings/applications/appinfo/AppButtonsPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/applications/appinfo/AppButtonsPreferenceControllerTest.java @@ -46,6 +46,7 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.os.RemoteException; import android.os.UserManager; +import android.util.ArraySet; import android.view.View; import com.android.settings.R; @@ -58,6 +59,8 @@ import com.android.settingslib.applications.instantapps.InstantAppDataProvider; import com.android.settingslib.core.lifecycle.Lifecycle; import com.android.settingslib.widget.ActionButtonsPreference; +import java.util.Set; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -67,6 +70,10 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.stubbing.Answer; import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; +import org.robolectric.annotation.Resetter; import org.robolectric.util.ReflectionHelpers; @RunWith(RobolectricTestRunner.class) @@ -149,6 +156,11 @@ public class AppButtonsPreferenceControllerTest { doAnswer(callable).when(mFragment).startActivityForResult(captor.capture(), anyInt()); } + @After + public void tearDown() { + ShadowAppUtils.reset(); + } + @Test public void retrieveAppEntry_hasAppEntry_notNull() throws PackageManager.NameNotFoundException { @@ -212,15 +224,9 @@ public class AppButtonsPreferenceControllerTest { } @Test + @Config(shadows = ShadowAppUtils.class) public void isAvailable_nonInstantApp() { mController.mAppEntry = mAppEntry; - ReflectionHelpers.setStaticField(AppUtils.class, "sInstantAppDataProvider", - new InstantAppDataProvider() { - @Override - public boolean isInstantApp(ApplicationInfo info) { - return false; - } - }); assertThat(mController.isAvailable()).isTrue(); } @@ -436,6 +442,14 @@ public class AppButtonsPreferenceControllerTest { // Should not crash in this method } + @Test + @Config(shadows = ShadowAppUtils.class) + public void getAvailabilityStatus_systemModule() { + ShadowAppUtils.addHiddenModule(mController.mPackageName); + assertThat(mController.getAvailabilityStatus()).isEqualTo( + AppButtonsPreferenceController.DISABLED_FOR_USER); + } + /** * The test fragment which implements * {@link ButtonActionDialogFragment.AppButtonsDialogListener} @@ -477,4 +491,29 @@ public class AppButtonsPreferenceControllerTest { priority, false /* isStatic */); } + + @Implements(AppUtils.class) + public static class ShadowAppUtils { + + public static Set sSystemModules = new ArraySet<>(); + + @Resetter + public static void reset() { + sSystemModules.clear(); + } + + public static void addHiddenModule(String pkg) { + sSystemModules.add(pkg); + } + + @Implementation + protected static boolean isInstant(ApplicationInfo info) { + return false; + } + + @Implementation + protected static boolean isSystemModule(Context context, String packageName) { + return sSystemModules.contains(packageName); + } + } } From 12731cce3dfa7ac40f356cc9927219a2550cb068 Mon Sep 17 00:00:00 2001 From: Salvador Martinez Date: Fri, 17 May 2019 15:36:55 -0700 Subject: [PATCH 2/2] Break infinite refresh loop in battery estimates There was an infinite loop that could occur in BatterySaverUtils what was caused by battery saver utils updating the battery estimate which then told the page to check for an estimate and then it would repeat from there. This cleans up the logic in that loop slightly to prevent it from refreshing more than is necessary. Test: atest BatteryUtilsTest Bug: 132751712 Change-Id: I918484747ecd9735315570ad608489e0f61d7578 --- .../settings/fuelgauge/BatteryUtils.java | 31 +++++++++++++------ .../settings/fuelgauge/BatteryUtilsTest.java | 14 +++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/com/android/settings/fuelgauge/BatteryUtils.java b/src/com/android/settings/fuelgauge/BatteryUtils.java index b293695ab82..788403ea188 100644 --- a/src/com/android/settings/fuelgauge/BatteryUtils.java +++ b/src/com/android/settings/fuelgauge/BatteryUtils.java @@ -55,6 +55,8 @@ import com.android.settingslib.utils.ThreadUtils; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.time.Duration; +import java.time.Instant; import java.util.Collections; import java.util.Comparator; import java.util.List; @@ -450,16 +452,10 @@ public class BatteryUtils { SystemClock.elapsedRealtime()); final BatteryStats stats = statsHelper.getStats(); BatteryInfo batteryInfo; - Estimate estimate = null; - // Get enhanced prediction if available - if (mPowerUsageFeatureProvider != null && - mPowerUsageFeatureProvider.isEnhancedBatteryPredictionEnabled(mContext)) { - estimate = mPowerUsageFeatureProvider.getEnhancedBatteryPrediction(mContext); - } + Estimate estimate = getEnhancedEstimate(); - if (estimate != null) { - Estimate.storeCachedEstimate(mContext, estimate); - } else { + // couldn't get estimate from cache or provider, use fallback + if (estimate == null) { estimate = new Estimate( PowerUtil.convertUsToMs(stats.computeBatteryTimeRemaining(elapsedRealtimeUs)), false /* isBasedOnUsage */, @@ -474,6 +470,23 @@ public class BatteryUtils { return batteryInfo; } + @VisibleForTesting + Estimate getEnhancedEstimate() { + Estimate estimate = null; + // Get enhanced prediction if available + if (Duration.between(Estimate.getLastCacheUpdateTime(mContext), Instant.now()) + .compareTo(Duration.ofSeconds(10)) < 0) { + estimate = Estimate.getCachedEstimateIfAvailable(mContext); + } else if (mPowerUsageFeatureProvider != null && + mPowerUsageFeatureProvider.isEnhancedBatteryPredictionEnabled(mContext)) { + estimate = mPowerUsageFeatureProvider.getEnhancedBatteryPrediction(mContext); + if (estimate != null) { + Estimate.storeCachedEstimate(mContext, estimate); + } + } + return estimate; + } + /** * Find the {@link BatterySipper} with the corresponding {@link BatterySipper.DrainType} */ diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java index a60460b8ce2..aa07e04e0c8 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java @@ -62,6 +62,7 @@ import com.android.settings.fuelgauge.batterytip.AnomalyInfo; import com.android.settings.fuelgauge.batterytip.BatteryDatabaseManager; import com.android.settings.testutils.FakeFeatureFactory; import com.android.settings.testutils.shadow.ShadowThreadUtils; +import com.android.settingslib.fuelgauge.Estimate; import com.android.settingslib.fuelgauge.PowerWhitelistBackend; import org.junit.Before; @@ -728,4 +729,17 @@ public class BatteryUtilsTest { //Should not crash assertThat(mBatteryUtils.getBatteryInfo(mBatteryStatsHelper, TAG)).isNotNull(); } + + @Test + public void getEnhancedEstimate_doesNotUpdateCache_ifEstimateFresh() { + Estimate estimate = new Estimate(1000, true, 1000); + Estimate.storeCachedEstimate(mContext, estimate); + + estimate = mBatteryUtils.getEnhancedEstimate(); + + // only pass if estimate has not changed + assertThat(estimate).isNotNull(); + assertThat(estimate.isBasedOnUsage()).isTrue(); + assertThat(estimate.getAverageDischargeTime()).isEqualTo(1000); + } }