diff --git a/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java b/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java index dbbafe6bdf6..ffbd2d90e4e 100644 --- a/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java +++ b/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java @@ -189,6 +189,8 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll mPrefContext = screen.getContext(); mAppListPrefGroup = screen.findPreference(mPreferenceKey); mAppListPrefGroup.setOrderingAsAdded(false); + mAppListPrefGroup.setTitle( + mPrefContext.getString(R.string.battery_app_usage_for_past_24)); mFooterPreference = screen.findPreference(KEY_FOOTER_PREF); // Removes footer first until usage data is loaded to avoid flashing. if (mFooterPreference != null) { @@ -216,15 +218,6 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll final BatteryHistEntry histEntry = diffEntry.mBatteryHistEntry; final String packageName = histEntry.mPackageName; final boolean isAppEntry = histEntry.isAppEntry(); - // Checks whether the package is installed or not. - boolean isValidPackage = true; - if (isAppEntry) { - if (mBatteryUtils == null) { - mBatteryUtils = BatteryUtils.getInstance(mPrefContext); - } - isValidPackage = mBatteryUtils.getPackageUid(packageName) - != BatteryUtils.UID_NULL; - } mMetricsFeatureProvider.action( mPrefContext, isAppEntry @@ -233,15 +226,12 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll new Pair(ConvertUtils.METRIC_KEY_PACKAGE, packageName), new Pair(ConvertUtils.METRIC_KEY_BATTERY_LEVEL, histEntry.mBatteryLevel), new Pair(ConvertUtils.METRIC_KEY_BATTERY_USAGE, powerPref.getPercent())); - Log.d(TAG, String.format("handleClick() label=%s key=%s isValid:%b\n%s", - diffEntry.getAppLabel(), histEntry.getKey(), isValidPackage, histEntry)); - if (isValidPackage) { - AdvancedPowerUsageDetail.startBatteryDetailPage( + Log.d(TAG, String.format("handleClick() label=%s key=%s enntry=\n%s", + diffEntry.getAppLabel(), histEntry.getKey(), histEntry)); + AdvancedPowerUsageDetail.startBatteryDetailPage( mActivity, mFragment, diffEntry, powerPref.getPercent(), isValidToShowSummary(packageName), getSlotInformation()); - return true; - } - return false; + return true; } @Override @@ -434,6 +424,7 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll pref.setSingleLineTitle(true); // Sets the BatteryDiffEntry to preference for launching detailed page. pref.setBatteryDiffEntry(entry); + pref.setEnabled(entry.validForRestriction()); setPreferenceSummary(pref, entry); if (!isAdded) { mAppListPrefGroup.addPreference(pref); diff --git a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java index 9db29f33fb7..c6b2d4542e2 100644 --- a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java +++ b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java @@ -39,6 +39,8 @@ public class BatteryDiffEntry { static Locale sCurrentLocale = null; // Caches app label and icon to improve loading performance. static final Map sResourceCache = new HashMap<>(); + // Whether a specific item is valid to launch restriction page? + static final Map sValidForRestriction = new HashMap<>(); /** A comparator for {@link BatteryDiffEntry} based on consumed percentage. */ public static final Comparator COMPARATOR = @@ -60,6 +62,7 @@ public class BatteryDiffEntry { @VisibleForTesting String mAppLabel = null; @VisibleForTesting Drawable mAppIcon = null; @VisibleForTesting boolean mIsLoaded = false; + @VisibleForTesting boolean mValidForRestriction = true; public BatteryDiffEntry( Context context, @@ -129,6 +132,12 @@ public class BatteryDiffEntry { ? mDefaultPackageName : mBatteryHistEntry.mPackageName; } + /** Whether this item is valid for users to launch restriction page? */ + public boolean validForRestriction() { + loadLabelAndIcon(); + return mValidForRestriction; + } + /** Whether the current BatteryDiffEntry is system component or not. */ public boolean isSystemEntry() { switch (mBatteryHistEntry.mConsumerType) { @@ -146,7 +155,29 @@ public class BatteryDiffEntry { if (mIsLoaded) { return; } + // Checks whether we have cached data or not first before fetching. + final BatteryEntry.NameAndIcon nameAndIcon = getCache(); + if (nameAndIcon != null) { + mAppLabel = nameAndIcon.name; + mAppIcon = nameAndIcon.icon; + mAppIconId = nameAndIcon.iconId; + } + final Boolean validForRestriction = sValidForRestriction.get(getKey()); + if (validForRestriction != null) { + mValidForRestriction = validForRestriction; + } + // Both nameAndIcon and restriction configuration have cached data. + if (nameAndIcon != null && validForRestriction != null) { + Log.w(TAG, String.format("cannot find cache data nameAndIcon:%s " + + "validForRestriction:%s", nameAndIcon, validForRestriction)); + return; + } mIsLoaded = true; + + // Configures whether we can launch restriction page or not. + updateRestrictionFlagState(); + sValidForRestriction.put(getKey(), Boolean.valueOf(mValidForRestriction)); + // Loads application icon and label based on consumer type. switch (mBatteryHistEntry.mConsumerType) { case ConvertUtils.CONSUMER_TYPE_USER_BATTERY: @@ -156,6 +187,9 @@ public class BatteryDiffEntry { if (nameAndIconForUser != null) { mAppIcon = nameAndIconForUser.icon; mAppLabel = nameAndIconForUser.name; + sResourceCache.put( + getKey(), + new BatteryEntry.NameAndIcon(mAppLabel, mAppIcon, /*iconId=*/ 0)); } break; case ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY: @@ -168,15 +202,12 @@ public class BatteryDiffEntry { mAppIconId = nameAndIconForSystem.iconId; mAppIcon = mContext.getDrawable(nameAndIconForSystem.iconId); } + sResourceCache.put( + getKey(), + new BatteryEntry.NameAndIcon(mAppLabel, mAppIcon, mAppIconId)); } break; case ConvertUtils.CONSUMER_TYPE_UID_BATTERY: - final BatteryEntry.NameAndIcon nameAndIcon = getCache(); - if (nameAndIcon != null) { - mAppLabel = nameAndIcon.name; - mAppIcon = nameAndIcon.icon; - break; - } loadNameAndIconForUid(); // Uses application default icon if we cannot find it from package. if (mAppIcon == null) { @@ -186,13 +217,47 @@ public class BatteryDiffEntry { mAppIcon = getBadgeIconForUser(mAppIcon); if (mAppLabel != null || mAppIcon != null) { sResourceCache.put( - mBatteryHistEntry.getKey(), + getKey(), new BatteryEntry.NameAndIcon(mAppLabel, mAppIcon, /*iconId=*/ 0)); } break; } } + @VisibleForTesting + String getKey() { + return mBatteryHistEntry.getKey(); + } + + @VisibleForTesting + void updateRestrictionFlagState() { + mValidForRestriction = true; + if (!mBatteryHistEntry.isAppEntry()) { + return; + } + final boolean isValidPackage = + BatteryUtils.getInstance(mContext).getPackageUid(getPackageName()) + != BatteryUtils.UID_NULL; + if (!isValidPackage) { + mValidForRestriction = false; + return; + } + try { + mValidForRestriction = + mContext.getPackageManager().getPackageInfo( + getPackageName(), + PackageManager.MATCH_DISABLED_COMPONENTS + | PackageManager.MATCH_ANY_USER + | PackageManager.GET_SIGNATURES + | PackageManager.GET_PERMISSIONS) + != null; + } catch (Exception e) { + Log.e(TAG, String.format("getPackageInfo() error %s for package=%s", + e.getCause(), getPackageName())); + mValidForRestriction = false; + } + } + private BatteryEntry.NameAndIcon getCache() { final Locale locale = Locale.getDefault(); if (sCurrentLocale != locale) { @@ -201,7 +266,7 @@ public class BatteryDiffEntry { sCurrentLocale = locale; clearCache(); } - return sResourceCache.get(mBatteryHistEntry.getKey()); + return sResourceCache.get(getKey()); } private void loadNameAndIconForUid() { @@ -258,7 +323,8 @@ public class BatteryDiffEntry { public String toString() { final StringBuilder builder = new StringBuilder() .append("BatteryDiffEntry{") - .append("\n\tname=" + mAppLabel) + .append(String.format("\n\tname=%s restrictable=%b", + mAppLabel, mValidForRestriction)) .append(String.format("\n\tconsume=%.2f%% %f/%f", mPercentOfTotal, mConsumePower, mTotalConsumePower)) .append(String.format("\n\tforeground:%s background:%s", @@ -274,6 +340,7 @@ public class BatteryDiffEntry { static void clearCache() { sResourceCache.clear(); + sValidForRestriction.clear(); } private Drawable getBadgeIconForUser(Drawable icon) { diff --git a/src/com/android/settings/fuelgauge/BatteryEntry.java b/src/com/android/settings/fuelgauge/BatteryEntry.java index 636d2651a8d..125409cd84f 100644 --- a/src/com/android/settings/fuelgauge/BatteryEntry.java +++ b/src/com/android/settings/fuelgauge/BatteryEntry.java @@ -241,7 +241,10 @@ public class BatteryEntry { mBatteryConsumer = null; mIsHidden = false; mPowerComponentId = powerComponentId; - mConsumedPower = devicePowerMah; + mConsumedPower = + powerComponentId == BatteryConsumer.POWER_COMPONENT_SCREEN + ? devicePowerMah + : devicePowerMah - appsPowerMah; mUsageDurationMs = usageDurationMs; mConsumerType = ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY; @@ -264,8 +267,10 @@ public class BatteryEntry { iconId = R.drawable.ic_power_system; icon = context.getDrawable(iconId); name = powerComponentName; - - mConsumedPower = devicePowerMah; + mConsumedPower = + powerComponentId == BatteryConsumer.POWER_COMPONENT_SCREEN + ? devicePowerMah + : devicePowerMah - appsPowerMah; mConsumerType = ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY; } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java index 9e2f65d6f3a..606dc194239 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java @@ -307,6 +307,7 @@ public final class BatteryChartPreferenceControllerTest { doReturn(appLabel).when(mBatteryDiffEntry).getAppLabel(); doReturn(PREF_KEY).when(mBatteryHistEntry).getKey(); doReturn(null).when(mAppListGroup).findPreference(PREF_KEY); + doReturn(false).when(mBatteryDiffEntry).validForRestriction(); mBatteryChartPreferenceController.addPreferenceToScreen( Arrays.asList(mBatteryDiffEntry)); @@ -324,6 +325,7 @@ public final class BatteryChartPreferenceControllerTest { assertThat(pref.getOrder()).isEqualTo(1); assertThat(pref.getBatteryDiffEntry()).isSameInstanceAs(mBatteryDiffEntry); assertThat(pref.isSingleLineTitle()).isTrue(); + assertThat(pref.isEnabled()).isFalse(); } @Test @@ -353,7 +355,7 @@ public final class BatteryChartPreferenceControllerTest { } @Test - public void testHandlePreferenceTreeClick_validPackageName_returnTrue() { + public void testHandlePreferenceTreeClick_forAppEntry_returnTrue() { doReturn(false).when(mBatteryHistEntry).isAppEntry(); doReturn(mBatteryDiffEntry).when(mPowerGaugePreference).getBatteryDiffEntry(); @@ -371,15 +373,13 @@ public final class BatteryChartPreferenceControllerTest { } @Test - public void testHandlePreferenceTreeClick_appEntryWithInvalidPackage_returnFalse() { + public void testHandlePreferenceTreeClick_forSystemEntry_returnTrue() { mBatteryChartPreferenceController.mBatteryUtils = mBatteryUtils; doReturn(true).when(mBatteryHistEntry).isAppEntry(); - doReturn(BatteryUtils.UID_NULL).when(mBatteryUtils) - .getPackageUid(mBatteryHistEntry.mPackageName); doReturn(mBatteryDiffEntry).when(mPowerGaugePreference).getBatteryDiffEntry(); assertThat(mBatteryChartPreferenceController.handlePreferenceTreeClick( - mPowerGaugePreference)).isFalse(); + mPowerGaugePreference)).isTrue(); verify(mMetricsFeatureProvider) .action( mContext, diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java index 586016399b3..0df53f1be41 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java @@ -17,12 +17,15 @@ package com.android.settings.fuelgauge; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import android.content.ContentValues; import android.content.Context; import android.content.pm.ApplicationInfo; +import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.graphics.drawable.Drawable; import android.os.BatteryConsumer; @@ -56,11 +59,13 @@ public final class BatteryDiffEntryTest { @Mock private Drawable mockDrawable2; @Mock private Drawable mockBadgedDrawable; @Mock private BatteryHistEntry mBatteryHistEntry; + @Mock private PackageInfo mockPackageInfo; @Before public void setUp() { MockitoAnnotations.initMocks(this); mContext = spy(RuntimeEnvironment.application); + doReturn(mContext).when(mContext).getApplicationContext(); doReturn(mockUserManager).when(mContext).getSystemService(UserManager.class); doReturn(mockPackageManager).when(mContext).getPackageManager(); BatteryDiffEntry.clearCache(); @@ -110,6 +115,7 @@ public final class BatteryDiffEntryTest { @Test public void testLoadLabelAndIcon_forSystemBattery_returnExpectedResult() { + final String expectedName = "Ambient display"; // Generates fake testing data. final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY); @@ -119,13 +125,22 @@ public final class BatteryDiffEntryTest { final BatteryDiffEntry entry = createBatteryDiffEntry(10, batteryHistEntry); - assertThat(entry.getAppLabel()).isEqualTo("Ambient display"); + assertThat(entry.getAppLabel()).isEqualTo(expectedName); assertThat(entry.getAppIconId()).isEqualTo(R.drawable.ic_settings_aod); - assertThat(BatteryDiffEntry.sResourceCache).isEmpty(); + assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); + // Verifies the app label in the cache. + final BatteryEntry.NameAndIcon nameAndIcon = + BatteryDiffEntry.sResourceCache.get(entry.getKey()); + assertThat(nameAndIcon.name).isEqualTo(expectedName); + assertThat(nameAndIcon.iconId).isEqualTo(R.drawable.ic_settings_aod); + // Verifies the restrictable flag in the cache. + assertThat(entry.mValidForRestriction).isTrue(); + assertThat(BatteryDiffEntry.sValidForRestriction.get(entry.getKey())).isTrue(); } @Test public void testLoadLabelAndIcon_forUserBattery_returnExpectedResult() { + final String expectedName = "Removed user"; doReturn(null).when(mockUserManager).getUserInfo(1001); // Generates fake testing data. final ContentValues values = getContentValuesWithType( @@ -135,10 +150,18 @@ public final class BatteryDiffEntryTest { final BatteryDiffEntry entry = createBatteryDiffEntry(10, batteryHistEntry); - assertThat(entry.getAppLabel()).isEqualTo("Removed user"); + assertThat(entry.getAppLabel()).isEqualTo(expectedName); assertThat(entry.getAppIcon()).isNull(); assertThat(entry.getAppIconId()).isEqualTo(0); - assertThat(BatteryDiffEntry.sResourceCache).isEmpty(); + assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); + // Verifies the app label in the cache. + final BatteryEntry.NameAndIcon nameAndIcon = + BatteryDiffEntry.sResourceCache.get(entry.getKey()); + assertThat(nameAndIcon.name).isEqualTo(expectedName); + assertThat(nameAndIcon.iconId).isEqualTo(0); + // Verifies the restrictable flag in the cache. + assertThat(entry.mValidForRestriction).isTrue(); + assertThat(BatteryDiffEntry.sValidForRestriction.get(entry.getKey())).isTrue(); } @Test @@ -162,8 +185,11 @@ public final class BatteryDiffEntryTest { assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); // Verifies the app label in the cache. final BatteryEntry.NameAndIcon nameAndIcon = - BatteryDiffEntry.sResourceCache.get(batteryHistEntry.getKey()); + BatteryDiffEntry.sResourceCache.get(entry.getKey()); assertThat(nameAndIcon.name).isEqualTo(expectedAppLabel); + // Verifies the restrictable flag in the cache. + assertThat(entry.mValidForRestriction).isFalse(); + assertThat(BatteryDiffEntry.sValidForRestriction.get(entry.getKey())).isFalse(); } @Test @@ -179,7 +205,7 @@ public final class BatteryDiffEntryTest { assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); // Verifies the app label in the cache. final BatteryEntry.NameAndIcon nameAndIcon = - BatteryDiffEntry.sResourceCache.get(batteryHistEntry.getKey()); + BatteryDiffEntry.sResourceCache.get(entry.getKey()); assertThat(nameAndIcon.name).isEqualTo(expectedAppLabel); } @@ -225,10 +251,24 @@ public final class BatteryDiffEntryTest { assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); // Verifies the app label in the cache. final BatteryEntry.NameAndIcon nameAndIcon = - BatteryDiffEntry.sResourceCache.get(entry.mBatteryHistEntry.getKey()); + BatteryDiffEntry.sResourceCache.get(entry.getKey()); assertThat(nameAndIcon.icon).isEqualTo(mockBadgedDrawable); } + @Test + public void testClearCache_clearDataForResourcesAndFlags() { + BatteryDiffEntry.sResourceCache.put( + "fake application key", + new BatteryEntry.NameAndIcon("app label", null, /*iconId=*/ 0)); + BatteryDiffEntry.sValidForRestriction.put( + "fake application key", Boolean.valueOf(false)); + + BatteryDiffEntry.clearCache(); + + assertThat(BatteryDiffEntry.sResourceCache).isEmpty(); + assertThat(BatteryDiffEntry.sValidForRestriction).isEmpty(); + } + @Test public void testClearCache_switchLocale_clearCacheIconAndLabel() throws Exception { final int userId = UserHandle.getUserId(1001); @@ -248,7 +288,7 @@ public final class BatteryDiffEntryTest { assertThat(entry2.getAppIcon()).isEqualTo(mockDrawable2); // Verifies the cache is updated into the new drawable. final BatteryEntry.NameAndIcon nameAndIcon = - BatteryDiffEntry.sResourceCache.get(entry2.mBatteryHistEntry.getKey()); + BatteryDiffEntry.sResourceCache.get(entry2.getKey()); assertThat(nameAndIcon.icon).isEqualTo(mockDrawable2); } @@ -297,6 +337,40 @@ public final class BatteryDiffEntryTest { assertThat(entry.isSystemEntry()).isTrue(); } + @Test + public void testUpdateRestrictionFlagState_updateFlagAsExpected() throws Exception { + final String expectedAppLabel = "fake app label"; + final String fakePackageName = "com.fake.google.com"; + final ContentValues values = getContentValuesWithType( + ConvertUtils.CONSUMER_TYPE_UID_BATTERY); + values.put("uid", /*invalid uid*/ 10001); + values.put("packageName", fakePackageName); + final BatteryDiffEntry entry = + createBatteryDiffEntry(10, new BatteryHistEntry(values)); + + entry.updateRestrictionFlagState(); + // Sets false if the app entry cannot be found. + assertThat(entry.mValidForRestriction).isFalse(); + + doReturn(BatteryUtils.UID_NULL).when(mockPackageManager).getPackageUid( + entry.getPackageName(), PackageManager.GET_META_DATA); + entry.updateRestrictionFlagState(); + // Sets false if the app is invalid package name. + assertThat(entry.mValidForRestriction).isFalse(); + + doReturn(1000).when(mockPackageManager).getPackageUid( + entry.getPackageName(), PackageManager.GET_META_DATA); + entry.updateRestrictionFlagState(); + // Sets false if the app PackageInfo cannot be found. + assertThat(entry.mValidForRestriction).isFalse(); + + doReturn(mockPackageInfo).when(mockPackageManager).getPackageInfo( + eq(entry.getPackageName()), anyInt()); + entry.updateRestrictionFlagState(); + // Sets true if package is valid and PackageInfo can be found. + assertThat(entry.mValidForRestriction).isTrue(); + } + private BatteryDiffEntry createBatteryDiffEntry( int consumerType, long uid, boolean isHidden) { final ContentValues values = getContentValuesWithType(consumerType);