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
This commit is contained in:
ykhung
2021-05-25 14:34:50 +08:00
committed by YUKAI HUNG
parent f49190d769
commit 46bcdda9b9
5 changed files with 178 additions and 41 deletions

View File

@@ -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,16 +226,13 @@ 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) {
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;
}
@Override
public void onSelect(int trapezoidIndex) {
@@ -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);

View File

@@ -39,6 +39,8 @@ public class BatteryDiffEntry {
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 @@ 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) {

View File

@@ -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;
}

View File

@@ -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,

View File

@@ -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);