Disable app usage item if this item is not clickable
some items are not clickable to launch the restriction page in the
battery usage list, we will apply the disabled visual in the
preferrence item to improve the UX (avoid users click the item without
any action)
Bug: 188751551
Bug: 188663505
Test: make SettingsgRoboTests
Change-Id: Ib8925b8e191117543bb1c74d6d01191e3043fc73
diff --git a/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java b/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java
index dbbafe6..ffbd2d9 100644
--- a/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java
+++ b/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java
@@ -189,6 +189,8 @@
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 @@
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 @@
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 @@
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 9db29f3..c6b2d45 100644
--- a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java
+++ b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java
@@ -39,6 +39,8 @@
static Locale sCurrentLocale = null;
// Caches app label and icon to improve loading performance.
static final Map<String, BatteryEntry.NameAndIcon> sResourceCache = new HashMap<>();
+ // Whether a specific item is valid to launch restriction page?
+ static final Map<String, Boolean> sValidForRestriction = new HashMap<>();
/** A comparator for {@link BatteryDiffEntry} based on consumed percentage. */
public static final Comparator<BatteryDiffEntry> COMPARATOR =
@@ -60,6 +62,7 @@
@VisibleForTesting String mAppLabel = null;
@VisibleForTesting Drawable mAppIcon = null;
@VisibleForTesting boolean mIsLoaded = false;
+ @VisibleForTesting boolean mValidForRestriction = true;
public BatteryDiffEntry(
Context context,
@@ -129,6 +132,12 @@
? 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 @@
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 @@
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 @@
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 @@
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 @@
sCurrentLocale = locale;
clearCache();
}
- return sResourceCache.get(mBatteryHistEntry.getKey());
+ return sResourceCache.get(getKey());
}
private void loadNameAndIconForUid() {
@@ -258,7 +323,8 @@
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 @@
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 636d265..125409c 100644
--- a/src/com/android/settings/fuelgauge/BatteryEntry.java
+++ b/src/com/android/settings/fuelgauge/BatteryEntry.java
@@ -241,7 +241,10 @@
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 @@
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 9e2f65d..606dc19 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 @@
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 @@
assertThat(pref.getOrder()).isEqualTo(1);
assertThat(pref.getBatteryDiffEntry()).isSameInstanceAs(mBatteryDiffEntry);
assertThat(pref.isSingleLineTitle()).isTrue();
+ assertThat(pref.isEnabled()).isFalse();
}
@Test
@@ -353,7 +355,7 @@
}
@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 @@
}
@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 5860163..0df53f1 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 @@
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 @@
@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 @@
@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 @@
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 @@
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 @@
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 @@
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,11 +251,25 @@
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);
doReturn(mockBadgedDrawable).when(mockUserManager)
@@ -248,7 +288,7 @@
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 @@
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);