diff --git a/src/com/android/settings/fuelgauge/BatteryAppListPreferenceController.java b/src/com/android/settings/fuelgauge/BatteryAppListPreferenceController.java index 5366ba19987..7741a979a92 100644 --- a/src/com/android/settings/fuelgauge/BatteryAppListPreferenceController.java +++ b/src/com/android/settings/fuelgauge/BatteryAppListPreferenceController.java @@ -352,15 +352,10 @@ public class BatteryAppListPreferenceController extends AbstractPreferenceContro @VisibleForTesting boolean shouldHideSipper(BatterySipper sipper) { - // Don't show hidden system module - final String packageName = mBatteryUtils.getPackageName(sipper.getUid()); - if (!TextUtils.isEmpty(packageName) - && AppUtils.isHiddenSystemModule(mContext, packageName)) { - return true; - } - // Don't show over-counted and unaccounted in any condition + // Don't show over-counted, unaccounted and hidden system module in any condition return sipper.drainType == BatterySipper.DrainType.OVERCOUNTED - || sipper.drainType == BatterySipper.DrainType.UNACCOUNTED; + || sipper.drainType == BatterySipper.DrainType.UNACCOUNTED + || mBatteryUtils.isHiddenSystemModule(sipper); } @VisibleForTesting diff --git a/src/com/android/settings/fuelgauge/BatteryUtils.java b/src/com/android/settings/fuelgauge/BatteryUtils.java index b293695ab82..e4e249c3725 100644 --- a/src/com/android/settings/fuelgauge/BatteryUtils.java +++ b/src/com/android/settings/fuelgauge/BatteryUtils.java @@ -47,6 +47,7 @@ import com.android.settings.fuelgauge.batterytip.AnomalyInfo; import com.android.settings.fuelgauge.batterytip.BatteryDatabaseManager; import com.android.settings.fuelgauge.batterytip.StatsManagerConfig; import com.android.settings.overlay.FeatureFactory; +import com.android.settingslib.applications.AppUtils; import com.android.settingslib.fuelgauge.Estimate; import com.android.settingslib.fuelgauge.EstimateKt; import com.android.settingslib.fuelgauge.PowerWhitelistBackend; @@ -187,8 +188,10 @@ public class BatteryUtils { && sipper.drainType != BatterySipper.DrainType.UNACCOUNTED && sipper.drainType != BatterySipper.DrainType.BLUETOOTH && sipper.drainType != BatterySipper.DrainType.WIFI - && sipper.drainType != BatterySipper.DrainType.IDLE) { - // Don't add it if it is overcounted, unaccounted, wifi, bluetooth, or screen + && sipper.drainType != BatterySipper.DrainType.IDLE + && !isHiddenSystemModule(sipper)) { + // Don't add it if it is overcounted, unaccounted, wifi, bluetooth, screen + // or hidden system modules proportionalSmearPowerMah += sipper.totalPowerMah; } } @@ -251,7 +254,27 @@ public class BatteryUtils { || drainType == BatterySipper.DrainType.WIFI || (sipper.totalPowerMah * SECONDS_IN_HOUR) < MIN_POWER_THRESHOLD_MILLI_AMP || mPowerUsageFeatureProvider.isTypeService(sipper) - || mPowerUsageFeatureProvider.isTypeSystem(sipper); + || mPowerUsageFeatureProvider.isTypeSystem(sipper) + || isHiddenSystemModule(sipper); + } + + /** + * Return {@code true} if one of packages in {@code sipper} is hidden system modules + */ + public boolean isHiddenSystemModule(BatterySipper sipper) { + if (sipper.uidObj == null) { + return false; + } + sipper.mPackages = mPackageManager.getPackagesForUid(sipper.getUid()); + if (sipper.mPackages != null) { + for (int i = 0, length = sipper.mPackages.length; i < length; i++) { + if (AppUtils.isHiddenSystemModule(mContext, sipper.mPackages[i])) { + return true; + } + } + } + + return false; } /** diff --git a/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetector.java b/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetector.java index 02af00c39a9..067046ca852 100644 --- a/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetector.java +++ b/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetector.java @@ -20,7 +20,6 @@ import static com.android.settings.Utils.SETTINGS_PACKAGE_NAME; import android.content.Context; import android.os.BatteryStats; -import android.text.format.DateUtils; import androidx.annotation.VisibleForTesting; @@ -72,22 +71,33 @@ public class HighUsageDetector implements BatteryTipDetector { if (mPolicy.highUsageEnabled && mDischarging) { parseBatteryData(); if (mDataParser.isDeviceHeavilyUsed() || mPolicy.testHighUsageTip) { - final List batterySippers = mBatteryStatsHelper.getUsageList(); - for (int i = 0, size = batterySippers.size(); i < size; i++) { - final BatterySipper batterySipper = batterySippers.get(i); - if (!mBatteryUtils.shouldHideSipper(batterySipper)) { - final long foregroundTimeMs = mBatteryUtils.getProcessTimeMs( - BatteryUtils.StatusType.FOREGROUND, batterySipper.uidObj, - BatteryStats.STATS_SINCE_CHARGED); - if (foregroundTimeMs >= DateUtils.MINUTE_IN_MILLIS) { - mHighUsageAppList.add(new AppInfo.Builder() - .setUid(batterySipper.getUid()) - .setPackageName( - mBatteryUtils.getPackageName(batterySipper.getUid())) - .setScreenOnTimeMs(foregroundTimeMs) - .build()); - } + final BatteryStats batteryStats = mBatteryStatsHelper.getStats(); + final List batterySippers + = new ArrayList<>(mBatteryStatsHelper.getUsageList()); + final double totalPower = mBatteryStatsHelper.getTotalPower(); + final int dischargeAmount = batteryStats != null + ? batteryStats.getDischargeAmount(BatteryStats.STATS_SINCE_CHARGED) + : 0; + + Collections.sort(batterySippers, + (sipper1, sipper2) -> Double.compare(sipper2.totalSmearedPowerMah, + sipper1.totalSmearedPowerMah)); + for (BatterySipper batterySipper : batterySippers) { + final double percent = mBatteryUtils.calculateBatteryPercent( + batterySipper.totalSmearedPowerMah, totalPower, 0, dischargeAmount); + if ((percent + 0.5f < 1f) || mBatteryUtils.shouldHideSipper(batterySipper)) { + // Don't show it if we should hide or usage percentage is lower than 1% + continue; } + mHighUsageAppList.add(new AppInfo.Builder() + .setUid(batterySipper.getUid()) + .setPackageName( + mBatteryUtils.getPackageName(batterySipper.getUid())) + .build()); + if (mHighUsageAppList.size() >= mPolicy.highUsageAppCount) { + break; + } + } // When in test mode, add an app if necessary @@ -97,10 +107,6 @@ public class HighUsageDetector implements BatteryTipDetector { .setScreenOnTimeMs(TimeUnit.HOURS.toMillis(3)) .build()); } - - Collections.sort(mHighUsageAppList, Collections.reverseOrder()); - mHighUsageAppList = mHighUsageAppList.subList(0, - Math.min(mPolicy.highUsageAppCount, mHighUsageAppList.size())); } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryAppListPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryAppListPreferenceControllerTest.java index 32829755a62..f8bcf016459 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryAppListPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryAppListPreferenceControllerTest.java @@ -20,12 +20,14 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; import android.content.Context; import android.content.pm.ModuleInfo; import android.content.pm.PackageManager; +import android.os.BatteryStats; import android.os.UserManager; import android.text.TextUtils; import android.text.format.DateUtils; @@ -94,6 +96,7 @@ public class BatteryAppListPreferenceControllerTest { when(mNormalBatterySipper.getPackages()).thenReturn(PACKAGE_NAMES); when(mNormalBatterySipper.getUid()).thenReturn(UID); mNormalBatterySipper.drainType = BatterySipper.DrainType.APP; + mNormalBatterySipper.uidObj = mock(BatteryStats.Uid.class); mPreferenceController = new BatteryAppListPreferenceController(mContext, KEY_APP_LIST, null, mSettingsActivity, mFragment); @@ -203,15 +206,7 @@ public class BatteryAppListPreferenceControllerTest { @Test public void testShouldHideSipper_hiddenSystemModule_returnTrue() { - ReflectionHelpers.setStaticField(ApplicationsState.class, "sInstance", null); - final String packageName = "test.hidden.module"; - final ModuleInfo moduleInfo = new ModuleInfo(); - moduleInfo.setPackageName(packageName); - moduleInfo.setHidden(true); - final List modules = new ArrayList<>(); - modules.add(moduleInfo); - when(mBatteryUtils.getPackageName(anyInt() /* uid */)).thenReturn(packageName); - when(mPackageManager.getInstalledModules(anyInt() /* flags */)).thenReturn(modules); + when(mBatteryUtils.isHiddenSystemModule(mNormalBatterySipper)).thenReturn(true); assertThat(mPreferenceController.shouldHideSipper(mNormalBatterySipper)).isTrue(); } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java index a60460b8ce2..9deaddd2e6a 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java @@ -367,6 +367,15 @@ public class BatteryUtilsTest { assertThat(mBatteryUtils.shouldHideSipper(mNormalBatterySipper)).isTrue(); } + @Test + public void testShouldHideSipper_hiddenSystemModule_ReturnTrue() { + mNormalBatterySipper.drainType = BatterySipper.DrainType.APP; + when(mNormalBatterySipper.getUid()).thenReturn(UID); + when(mBatteryUtils.isHiddenSystemModule(mNormalBatterySipper)).thenReturn(true); + + assertThat(mBatteryUtils.shouldHideSipper(mNormalBatterySipper)).isTrue(); + } + @Test public void testCalculateBatteryPercent() { assertThat(mBatteryUtils.calculateBatteryPercent(BATTERY_SYSTEM_USAGE, TOTAL_BATTERY_USAGE, diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetectorTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetectorTest.java index 7b9e5a5c73a..5c56f46c4bd 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetectorTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/detectors/HighUsageDetectorTest.java @@ -18,6 +18,7 @@ package com.android.settings.fuelgauge.batterytip.detectors; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -40,6 +41,7 @@ import com.android.settings.fuelgauge.batterytip.tips.HighUsageTip; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Answers; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.RobolectricTestRunner; @@ -52,22 +54,25 @@ import java.util.List; @RunWith(RobolectricTestRunner.class) public class HighUsageDetectorTest { private static final int UID_HIGH = 123; - private static final int UID_ZERO = 345; - private static final long SCREEN_ON_TIME_MS = DateUtils.HOUR_IN_MILLIS; + private static final int UID_LOW = 345; + private static final double POWER_HIGH = 20000; + private static final double POWER_LOW = 10000; private Context mContext; - @Mock + @Mock(answer = Answers.RETURNS_DEEP_STUBS) private BatteryStatsHelper mBatteryStatsHelper; @Mock - private BatteryUtils mBatteryUtils; - @Mock private BatterySipper mHighBatterySipper; @Mock - private BatterySipper mZeroBatterySipper; + private BatterySipper mLowBatterySipper; + @Mock + private BatterySipper mSystemBatterySipper; @Mock private HighUsageDataParser mDataParser; - private AppInfo mAppInfo; + private AppInfo mHighAppInfo; + private AppInfo mLowAppInfo; private BatteryTipPolicy mPolicy; + private BatteryUtils mBatteryUtils; private HighUsageDetector mHighUsageDetector; private List mUsageList; @@ -77,28 +82,37 @@ public class HighUsageDetectorTest { mContext = RuntimeEnvironment.application; mPolicy = spy(new BatteryTipPolicy(mContext)); + mBatteryUtils = spy(BatteryUtils.getInstance(mContext)); mHighUsageDetector = spy(new HighUsageDetector(mContext, mPolicy, mBatteryStatsHelper, true /* mDischarging */)); mHighUsageDetector.mBatteryUtils = mBatteryUtils; mHighUsageDetector.mDataParser = mDataParser; doNothing().when(mHighUsageDetector).parseBatteryData(); doReturn(UID_HIGH).when(mHighBatterySipper).getUid(); + doReturn(UID_LOW).when(mLowBatterySipper).getUid(); mHighBatterySipper.uidObj = mock(BatteryStats.Uid.class); - mZeroBatterySipper.uidObj = mock(BatteryStats.Uid.class); - doReturn(UID_ZERO).when(mZeroBatterySipper).getUid(); - mAppInfo = new AppInfo.Builder() + mHighBatterySipper.drainType = BatterySipper.DrainType.APP; + mHighBatterySipper.totalSmearedPowerMah = POWER_HIGH; + mLowBatterySipper.uidObj = mock(BatteryStats.Uid.class); + mLowBatterySipper.drainType = BatterySipper.DrainType.APP; + mLowBatterySipper.totalSmearedPowerMah = POWER_LOW; + when(mBatteryUtils.shouldHideSipper(mSystemBatterySipper)).thenReturn(true); + when(mBatteryUtils.shouldHideSipper(mHighBatterySipper)).thenReturn(false); + when(mBatteryUtils.shouldHideSipper(mLowBatterySipper)).thenReturn(false); + when(mBatteryStatsHelper.getStats().getDischargeAmount(anyInt())).thenReturn(100); + when(mBatteryStatsHelper.getTotalPower()).thenReturn(POWER_HIGH + POWER_LOW); + + + mHighAppInfo = new AppInfo.Builder() .setUid(UID_HIGH) - .setScreenOnTimeMs(SCREEN_ON_TIME_MS) + .build(); + mLowAppInfo = new AppInfo.Builder() + .setUid(UID_LOW) .build(); - doReturn(SCREEN_ON_TIME_MS).when(mBatteryUtils).getProcessTimeMs( - BatteryUtils.StatusType.FOREGROUND, mHighBatterySipper.uidObj, - BatteryStats.STATS_SINCE_CHARGED); - doReturn(0L).when(mBatteryUtils).getProcessTimeMs( - BatteryUtils.StatusType.FOREGROUND, mZeroBatterySipper.uidObj, - BatteryStats.STATS_SINCE_CHARGED); - mUsageList = new ArrayList<>(); + mUsageList.add(mSystemBatterySipper); + mUsageList.add(mLowBatterySipper); mUsageList.add(mHighBatterySipper); when(mBatteryStatsHelper.getUsageList()).thenReturn(mUsageList); } @@ -128,21 +142,15 @@ public class HighUsageDetectorTest { } @Test - public void testDetect_containsHighUsageApp_tipVisible() { + public void testDetect_containsHighUsageApp_tipVisibleAndSorted() { doReturn(true).when(mDataParser).isDeviceHeavilyUsed(); final HighUsageTip highUsageTip = (HighUsageTip) mHighUsageDetector.detect(); assertThat(highUsageTip.isVisible()).isTrue(); - assertThat(highUsageTip.getHighUsageAppList()).containsExactly(mAppInfo); - } - @Test - public void testDetect_containsHighUsageApp_removeZeroOne() { - doReturn(true).when(mDataParser).isDeviceHeavilyUsed(); - mUsageList.add(mZeroBatterySipper); - - final HighUsageTip highUsageTip = (HighUsageTip) mHighUsageDetector.detect(); - assertThat(highUsageTip.isVisible()).isTrue(); - assertThat(highUsageTip.getHighUsageAppList()).containsExactly(mAppInfo); + // Contain two appInfo and large one comes first + final List appInfos = highUsageTip.getHighUsageAppList(); + assertThat(appInfos).containsExactly(mLowAppInfo, mHighAppInfo); + assertThat(appInfos.get(0)).isEqualTo(mHighAppInfo); } }