Merge "Fix issues in battery usage accounting" into qt-dev

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
+        // Don't show over-counted, unaccounted and hidden system module in any condition
-        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
         return sipper.drainType == BatterySipper.DrainType.OVERCOUNTED
-                || sipper.drainType == BatterySipper.DrainType.UNACCOUNTED;
+                || sipper.drainType == BatterySipper.DrainType.UNACCOUNTED
+                || mBatteryUtils.isHiddenSystemModule(sipper);
     }
 
     @VisibleForTesting

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;
@@ -189,8 +190,10 @@ public class BatteryUtils {
                         && sipper.drainType != BatterySipper.DrainType.UNACCOUNTED
                         && sipper.drainType != BatterySipper.DrainType.BLUETOOTH
                         && sipper.drainType != BatterySipper.DrainType.WIFI
-                        && sipper.drainType != BatterySipper.DrainType.IDLE) {
+                        && sipper.drainType != BatterySipper.DrainType.IDLE
-                    // Don't add it if it is overcounted, unaccounted, wifi, bluetooth, or screen
+                        && !isHiddenSystemModule(sipper)) {
+                    // Don't add it if it is overcounted, unaccounted, wifi, bluetooth, screen
+                    // or hidden system modules
                     proportionalSmearPowerMah += sipper.totalPowerMah;
                 }
             }
@@ -253,7 +256,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;
     }
 
     /**

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<BatterySipper> batterySippers = mBatteryStatsHelper.getUsageList();
+                final BatteryStats batteryStats = mBatteryStatsHelper.getStats();
-                for (int i = 0, size = batterySippers.size(); i < size; i++) {
+                final List<BatterySipper> batterySippers
-                    final BatterySipper batterySipper = batterySippers.get(i);
+                        = new ArrayList<>(mBatteryStatsHelper.getUsageList());
-                    if (!mBatteryUtils.shouldHideSipper(batterySipper)) {
+                final double totalPower = mBatteryStatsHelper.getTotalPower();
-                        final long foregroundTimeMs = mBatteryUtils.getProcessTimeMs(
+                final int dischargeAmount = batteryStats != null
-                                BatteryUtils.StatusType.FOREGROUND, batterySipper.uidObj,
+                        ? batteryStats.getDischargeAmount(BatteryStats.STATS_SINCE_CHARGED)
-                                BatteryStats.STATS_SINCE_CHARGED);
+                        : 0;
-                        if (foregroundTimeMs >= DateUtils.MINUTE_IN_MILLIS) {
+
-                            mHighUsageAppList.add(new AppInfo.Builder()
+                Collections.sort(batterySippers,
-                                    .setUid(batterySipper.getUid())
+                        (sipper1, sipper2) -> Double.compare(sipper2.totalSmearedPowerMah,
-                                    .setPackageName(
+                                sipper1.totalSmearedPowerMah));
-                                            mBatteryUtils.getPackageName(batterySipper.getUid()))
+                for (BatterySipper batterySipper : batterySippers) {
-                                    .setScreenOnTimeMs(foregroundTimeMs)
+                    final double percent = mBatteryUtils.calculateBatteryPercent(
-                                    .build());
+                            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()));
             }
         }
 

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);
+        when(mBatteryUtils.isHiddenSystemModule(mNormalBatterySipper)).thenReturn(true);
-        final String packageName = "test.hidden.module";
-        final ModuleInfo moduleInfo = new ModuleInfo();
-        moduleInfo.setPackageName(packageName);
-        moduleInfo.setHidden(true);
-        final List<ModuleInfo> modules = new ArrayList<>();
-        modules.add(moduleInfo);
-        when(mBatteryUtils.getPackageName(anyInt() /* uid */)).thenReturn(packageName);
-        when(mPackageManager.getInstalledModules(anyInt() /* flags */)).thenReturn(modules);
 
         assertThat(mPreferenceController.shouldHideSipper(mNormalBatterySipper)).isTrue();
     }

tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java

@@ -368,6 +368,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,

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 int UID_LOW = 345;
-    private static final long SCREEN_ON_TIME_MS = DateUtils.HOUR_IN_MILLIS;
+    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<BatterySipper> 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);
+        mHighBatterySipper.drainType = BatterySipper.DrainType.APP;
-        doReturn(UID_ZERO).when(mZeroBatterySipper).getUid();
+        mHighBatterySipper.totalSmearedPowerMah = POWER_HIGH;
-        mAppInfo = new AppInfo.Builder()
+        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
+        // Contain two appInfo and large one comes first
-    public void testDetect_containsHighUsageApp_removeZeroOne() {
+        final List<AppInfo> appInfos = highUsageTip.getHighUsageAppList();
-        doReturn(true).when(mDataParser).isDeviceHeavilyUsed();
+        assertThat(appInfos).containsExactly(mLowAppInfo, mHighAppInfo);
-        mUsageList.add(mZeroBatterySipper);
+        assertThat(appInfos.get(0)).isEqualTo(mHighAppInfo);
-
-        final HighUsageTip highUsageTip = (HighUsageTip) mHighUsageDetector.detect();
-        assertThat(highUsageTip.isVisible()).isTrue();
-        assertThat(highUsageTip.getHighUsageAppList()).containsExactly(mAppInfo);
     }
 }