Add cache mechanism to improve icon and label loading performance

Bug: 185207505
Test: make SettingsRoboTests
Test: make SettingsGoogleRoboTests
Change-Id: I73dba5e40783f9ef4cfc0c4c33ea56b12754535d

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

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<String, BatteryEntry.NameAndIcon> sResourceCache = new HashMap<>();
+
     /** A comparator for {@link BatteryDiffEntry} based on consumed percentage. */
     public static final Comparator<BatteryDiffEntry> 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 !=
+        return mAppIcon;
-                ConvertUtils.CONSUMER_TYPE_UID_BATTERY) {
-            return mAppIcon;
-        }
-        // Returns default application icon if UID_BATTERY icon is null.
-        return mAppIcon == null
-            ? mContext.getPackageManager().getDefaultActivityIcon()
-            : 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> T getNonNull(T originalObj, T newObj) {
         return newObj != null ? newObj : originalObj;
     }

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

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() {
+    public void testGetAppIcon_uidConsumerWithNullIcon_returnDefaultActivityIcon()
-        final ContentValues values = getContentValuesWithType(
+            throws Exception {
-            ConvertUtils.CONSUMER_TYPE_UID_BATTERY);
+        final BatteryDiffEntry entry = createBatteryDiffEntry(mockDrawable);
-        final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values);
-        doReturn(mockDrawable).when(mockPackageManager).getDefaultActivityIcon();
 
-        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);
+    }
 }

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