diff --git a/src/com/android/settings/fuelgauge/BatteryChartView.java b/src/com/android/settings/fuelgauge/BatteryChartView.java index 5e4d9809aa9..26eb3e40c55 100644 --- a/src/com/android/settings/fuelgauge/BatteryChartView.java +++ b/src/com/android/settings/fuelgauge/BatteryChartView.java @@ -255,7 +255,8 @@ public class BatteryChartView extends AppCompatImageView implements View.OnClick private int getTrapezoidIndex(float x) { for (int index = 0; index < mTrapezoidSlot.length; index++) { final TrapezoidSlot slot = mTrapezoidSlot[index]; - if (x >= slot.mLeft && x <= slot.mRight) { + if (x >= slot.mLeft - mTrapezoidHOffset + && x <= slot.mRight + mTrapezoidHOffset) { return index; } } diff --git a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java index efc55545e36..da3825f0b80 100644 --- a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java +++ b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java @@ -26,11 +26,18 @@ import androidx.annotation.VisibleForTesting; import java.time.Duration; import java.util.Comparator; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; /** A container class to carry battery data in a specific time slot. */ public final class BatteryDiffEntry { private static final String TAG = "BatteryDiffEntry"; + static Locale sCurrentLocale = null; + // Caches app label and icon to improve loading performance. + static final Map sResourceCache = new HashMap<>(); + /** A comparator for {@link BatteryDiffEntry} based on consumed percentage. */ public static final Comparator COMPARATOR = (a, b) -> Double.compare(b.getPercentOfTotal(), a.getPercentOfTotal()); @@ -97,14 +104,7 @@ public final class BatteryDiffEntry { /** Gets the app icon {@link Drawable} for this entry. */ public Drawable getAppIcon() { loadLabelAndIcon(); - if (mBatteryHistEntry.mConsumerType != - ConvertUtils.CONSUMER_TYPE_UID_BATTERY) { - return mAppIcon; - } - // Returns default application icon if UID_BATTERY icon is null. - return mAppIcon == null - ? mContext.getPackageManager().getDefaultActivityIcon() - : mAppIcon; + return mAppIcon; } /** Gets the searching package name for UID battery type. */ @@ -140,11 +140,37 @@ public final class BatteryDiffEntry { } 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) { + mAppIcon = mContext.getPackageManager().getDefaultActivityIcon(); + } + if (mAppLabel != null && mAppIcon != null) { + sResourceCache.put( + mBatteryHistEntry.getKey(), + new BatteryEntry.NameAndIcon(mAppLabel, mAppIcon, /*iconId=*/ 0)); + } break; } } + private BatteryEntry.NameAndIcon getCache() { + final Locale locale = Locale.getDefault(); + if (sCurrentLocale != locale) { + Log.d(TAG, String.format("clearCache() locale is changed from %s to %s", + sCurrentLocale, locale)); + sCurrentLocale = locale; + clearCache(); + } + return sResourceCache.get(mBatteryHistEntry.getKey()); + } + private void loadNameAndIconForUid() { final String packageName = mBatteryHistEntry.mPackageName; final PackageManager packageManager = mContext.getPackageManager(); @@ -153,7 +179,9 @@ public final class BatteryDiffEntry { try { final ApplicationInfo appInfo = packageManager.getApplicationInfo(packageName, /*no flags*/ 0); - mAppLabel = packageManager.getApplicationLabel(appInfo).toString(); + if (appInfo != null) { + mAppLabel = packageManager.getApplicationLabel(appInfo).toString(); + } } catch (NameNotFoundException e) { Log.e(TAG, "failed to retrieve ApplicationInfo for: " + packageName); mAppLabel = packageName; @@ -203,6 +231,10 @@ public final class BatteryDiffEntry { return builder.toString(); } + static void clearCache() { + sResourceCache.clear(); + } + private static T getNonNull(T originalObj, T newObj) { return newObj != null ? newObj : originalObj; } diff --git a/src/com/android/settings/fuelgauge/BatteryHistEntry.java b/src/com/android/settings/fuelgauge/BatteryHistEntry.java index e8cbce5bfd5..9611de18ec0 100644 --- a/src/com/android/settings/fuelgauge/BatteryHistEntry.java +++ b/src/com/android/settings/fuelgauge/BatteryHistEntry.java @@ -65,6 +65,7 @@ public final class BatteryHistEntry { public final int mBatteryStatus; public final int mBatteryHealth; + private String mKey = null; private boolean mIsValidEntry = true; public BatteryHistEntry(ContentValues values) { @@ -114,7 +115,20 @@ public final class BatteryHistEntry { /** Gets an identifier to represent this {@link BatteryHistEntry}. */ public String getKey() { - return mPackageName + "-" + mUserId; + if (mKey == null) { + switch (mConsumerType) { + case ConvertUtils.CONSUMER_TYPE_UID_BATTERY: + mKey = Long.toString(mUid); + break; + case ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY: + mKey = "S|" + mDrainType; + break; + case ConvertUtils.CONSUMER_TYPE_USER_BATTERY: + mKey = "U|" + mUserId; + break; + } + } + return mKey; } @Override diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java index 0f7a40732f2..84ff07b3c2c 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java @@ -17,6 +17,7 @@ package com.android.settings.fuelgauge; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; @@ -39,6 +40,7 @@ import org.robolectric.RuntimeEnvironment; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Locale; @RunWith(RobolectricTestRunner.class) public final class BatteryDiffEntryTest { @@ -49,6 +51,7 @@ public final class BatteryDiffEntryTest { @Mock private PackageManager mockPackageManager; @Mock private UserManager mockUserManager; @Mock private Drawable mockDrawable; + @Mock private Drawable mockDrawable2; @Before public void setUp() { @@ -56,6 +59,7 @@ public final class BatteryDiffEntryTest { mContext = spy(RuntimeEnvironment.application); doReturn(mockUserManager).when(mContext).getSystemService(UserManager.class); doReturn(mockPackageManager).when(mContext).getPackageManager(); + BatteryDiffEntry.clearCache(); } @Test @@ -112,6 +116,7 @@ public final class BatteryDiffEntryTest { final BatteryDiffEntry entry = createBatteryDiffEntry(10, batteryHistEntry); assertThat(entry.getAppLabel()).isEqualTo("Ambient display"); + assertThat(BatteryDiffEntry.sResourceCache).isEmpty(); } @Test @@ -127,6 +132,7 @@ public final class BatteryDiffEntryTest { assertThat(entry.getAppLabel()).isEqualTo("Removed user"); assertThat(entry.getAppIcon()).isNull(); + assertThat(BatteryDiffEntry.sResourceCache).isEmpty(); } @Test @@ -146,17 +152,28 @@ public final class BatteryDiffEntryTest { final BatteryDiffEntry entry = createBatteryDiffEntry(10, batteryHistEntry); assertThat(entry.getAppLabel()).isEqualTo(expectedAppLabel); + assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); + // Verifies the app label in the cache. + final BatteryEntry.NameAndIcon nameAndIcon = + BatteryDiffEntry.sResourceCache.get(batteryHistEntry.getKey()); + assertThat(nameAndIcon.name).isEqualTo(expectedAppLabel); } @Test public void testGetAppLabel_loadDataFromPreDefinedNameAndUid() { + final String expectedAppLabel = "Android OS"; final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_UID_BATTERY); final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); final BatteryDiffEntry entry = createBatteryDiffEntry(10, batteryHistEntry); - assertThat(entry.getAppLabel()).isEqualTo("Android OS"); + assertThat(entry.getAppLabel()).isEqualTo(expectedAppLabel); + assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); + // Verifies the app label in the cache. + final BatteryEntry.NameAndIcon nameAndIcon = + BatteryDiffEntry.sResourceCache.get(batteryHistEntry.getKey()); + assertThat(nameAndIcon.name).isEqualTo(expectedAppLabel); } @Test @@ -171,6 +188,7 @@ public final class BatteryDiffEntryTest { entry.mIsLoaded = true; assertThat(entry.getAppLabel()).isEqualTo(expectedAppLabel); + assertThat(BatteryDiffEntry.sResourceCache).isEmpty(); } @Test @@ -184,20 +202,39 @@ public final class BatteryDiffEntryTest { entry.mIsLoaded = true; entry.mAppIcon = mockDrawable; assertThat(entry.getAppIcon()).isEqualTo(mockDrawable); + assertThat(BatteryDiffEntry.sResourceCache).isEmpty(); } @Test - public void testGetAppIcon_uidConsumerWithNullIcon_returnDefaultActivityIcon() { - final ContentValues values = getContentValuesWithType( - ConvertUtils.CONSUMER_TYPE_UID_BATTERY); - final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); - doReturn(mockDrawable).when(mockPackageManager).getDefaultActivityIcon(); + public void testGetAppIcon_uidConsumerWithNullIcon_returnDefaultActivityIcon() + throws Exception { + final BatteryDiffEntry entry = createBatteryDiffEntry(mockDrawable); - final BatteryDiffEntry entry = createBatteryDiffEntry(10, batteryHistEntry); - - entry.mIsLoaded = true; entry.mAppIcon = null; assertThat(entry.getAppIcon()).isEqualTo(mockDrawable); + assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); + // Verifies the app label in the cache. + final BatteryEntry.NameAndIcon nameAndIcon = + BatteryDiffEntry.sResourceCache.get(entry.mBatteryHistEntry.getKey()); + assertThat(nameAndIcon.icon).isEqualTo(mockDrawable); + } + + @Test + public void testClearCache_switchLocale_clearCacheIconAndLabel() throws Exception { + Locale.setDefault(new Locale("en_US")); + final BatteryDiffEntry entry1 = createBatteryDiffEntry(mockDrawable); + assertThat(entry1.getAppIcon()).isEqualTo(mockDrawable); + // Switch the locale into another one. + Locale.setDefault(new Locale("zh_TW")); + + final BatteryDiffEntry entry2 = createBatteryDiffEntry(mockDrawable2); + + // We should get new drawable without caching. + assertThat(entry2.getAppIcon()).isEqualTo(mockDrawable2); + // Verifies the cache is updated into the new drawable. + final BatteryEntry.NameAndIcon nameAndIcon = + BatteryDiffEntry.sResourceCache.get(entry2.mBatteryHistEntry.getKey()); + assertThat(nameAndIcon.icon).isEqualTo(mockDrawable2); } private BatteryDiffEntry createBatteryDiffEntry( @@ -217,4 +254,17 @@ public final class BatteryDiffEntryTest { values.put("consumerType", Integer.valueOf(consumerType)); return values; } + + private BatteryDiffEntry createBatteryDiffEntry(Drawable drawable) throws Exception { + final ContentValues values = getContentValuesWithType( + ConvertUtils.CONSUMER_TYPE_UID_BATTERY); + values.put("uid", 1001); + values.put("packageName", "com.a.b.c"); + final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); + doReturn(drawable).when(mockPackageManager).getDefaultActivityIcon(); + doReturn(null).when(mockPackageManager).getApplicationInfo("com.a.b.c", 0); + doReturn(new String[] {"com.a.b.c"}).when(mockPackageManager) + .getPackagesForUid(1001); + return createBatteryDiffEntry(10, batteryHistEntry); + } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryHistEntryTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryHistEntryTest.java index d97b0d22baf..72ccebe1b43 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryHistEntryTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryHistEntryTest.java @@ -137,6 +137,42 @@ public final class BatteryHistEntryTest { /*percentOfTotal=*/ 0.3); } + @Test + public void testGetKey_consumerUidType_returnExpectedString() { + final ContentValues values = getContentValuesWithType( + ConvertUtils.CONSUMER_TYPE_UID_BATTERY); + values.put("uid", 3); + final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); + + assertThat(batteryHistEntry.getKey()).isEqualTo("3"); + } + + @Test + public void testGetKey_consumerUserType_returnExpectedString() { + final ContentValues values = getContentValuesWithType( + ConvertUtils.CONSUMER_TYPE_USER_BATTERY); + values.put("userId", 2); + final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); + + assertThat(batteryHistEntry.getKey()).isEqualTo("U|2"); + } + + @Test + public void testGetKey_consumerSystemType_returnExpectedString() { + final ContentValues values = getContentValuesWithType( + ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY); + values.put("drainType", 1); + final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); + + assertThat(batteryHistEntry.getKey()).isEqualTo("S|1"); + } + + private static ContentValues getContentValuesWithType(int consumerType) { + final ContentValues values = new ContentValues(); + values.put("consumerType", Integer.valueOf(consumerType)); + return values; + } + private void assertBatteryHistEntry( BatteryHistEntry entry, int drainType, double percentOfTotal) { assertThat(entry.isValidEntry()).isTrue(); @@ -161,7 +197,5 @@ public final class BatteryHistEntryTest { .isEqualTo(BatteryManager.BATTERY_STATUS_FULL); assertThat(entry.mBatteryHealth) .isEqualTo(BatteryManager.BATTERY_HEALTH_COLD); - assertThat(entry.getKey()) - .isEqualTo("com.google.android.settings.battery-" + entry.mUserId); } }