Add protection for the ConcurrentModificationException

Fix: 310120803
Test: presubmit
Change-Id: I8526ed0c93570ba3e183b675b29bd9e9e1582f5a

src/com/android/settings/fuelgauge/batteryusage/BatteryDiffEntry.java

@@ -27,10 +27,12 @@ import android.util.ArrayMap;
 import android.util.Log;
 import android.util.Pair;
 
+import androidx.annotation.GuardedBy;
 import androidx.annotation.VisibleForTesting;
 
 import com.android.settings.R;
 import com.android.settings.fuelgauge.BatteryUtils;
+import com.android.settings.fuelgauge.batteryusage.BatteryEntry.NameAndIcon;
 import com.android.settingslib.utils.StringUtil;
 
 import java.util.Comparator;
@@ -40,16 +42,23 @@ import java.util.Map;
 /** A container class to carry battery data in a specific time slot. */
 public class BatteryDiffEntry {
     private static final String TAG = "BatteryDiffEntry";
+    private static final Object sResourceCacheLock = new Object();
+    private static final Object sPackageNameAndUidCacheLock = new Object();
+    private static final Object sValidForRestrictionLock = new Object();
 
     static Locale sCurrentLocale = null;
+
     // Caches app label and icon to improve loading performance.
-    static final Map<String, BatteryEntry.NameAndIcon> sResourceCache = new ArrayMap<>();
+    @GuardedBy("sResourceCacheLock")
+    static final Map<String, NameAndIcon> sResourceCache = new ArrayMap<>();
 
     // Caches package name and uid to improve loading performance.
+    @GuardedBy("sPackageNameAndUidCacheLock")
     static final Map<String, Integer> sPackageNameAndUidCache = new ArrayMap<>();
 
     // Whether a specific item is valid to launch restriction page?
     @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
+    @GuardedBy("sValidForRestrictionLock")
     static final Map<String, Boolean> sValidForRestriction = new ArrayMap<>();
 
     /** A comparator for {@link BatteryDiffEntry} based on the sorting key. */
@@ -304,12 +313,16 @@ public class BatteryDiffEntry {
     }
 
     private int getPackageUid(String packageName) {
-        if (sPackageNameAndUidCache.containsKey(packageName)) {
+        synchronized (sPackageNameAndUidCacheLock) {
-            return sPackageNameAndUidCache.get(packageName);
+            if (sPackageNameAndUidCache.containsKey(packageName)) {
+                return sPackageNameAndUidCache.get(packageName);
+            }
         }
 
         int uid = BatteryUtils.getInstance(mContext).getPackageUid(packageName);
-        sPackageNameAndUidCache.put(packageName, uid);
+        synchronized (sPackageNameAndUidCacheLock) {
+            sPackageNameAndUidCache.put(packageName, uid);
+        }
         return uid;
     }
 
@@ -318,13 +331,16 @@ public class BatteryDiffEntry {
             return;
         }
         // Checks whether we have cached data or not first before fetching.
-        final BatteryEntry.NameAndIcon nameAndIcon = getCache();
+        final NameAndIcon nameAndIcon = getCache();
         if (nameAndIcon != null) {
             mAppLabel = nameAndIcon.mName;
             mAppIcon = nameAndIcon.mIcon;
             mAppIconId = nameAndIcon.mIconId;
         }
-        final Boolean validForRestriction = sValidForRestriction.get(getKey());
+        Boolean validForRestriction = null;
+        synchronized (sValidForRestrictionLock) {
+            validForRestriction = sValidForRestriction.get(getKey());
+        }
         if (validForRestriction != null) {
             mValidForRestriction = validForRestriction;
         }
@@ -336,33 +352,34 @@ public class BatteryDiffEntry {
 
         // Configures whether we can launch restriction page or not.
         updateRestrictionFlagState();
-        sValidForRestriction.put(getKey(), Boolean.valueOf(mValidForRestriction));
+        synchronized (sValidForRestrictionLock) {
+            sValidForRestriction.put(getKey(), Boolean.valueOf(mValidForRestriction));
+        }
 
         if (getKey() != null && SPECIAL_ENTRY_MAP.containsKey(getKey())) {
             Pair<Integer, Integer> pair = SPECIAL_ENTRY_MAP.get(getKey());
             mAppLabel = mContext.getString(pair.first);
             mAppIconId = pair.second;
             mAppIcon = mContext.getDrawable(mAppIconId);
-            sResourceCache.put(
+            putResourceCache(getKey(), new NameAndIcon(mAppLabel, mAppIcon, mAppIconId));
-                    getKey(), new BatteryEntry.NameAndIcon(mAppLabel, mAppIcon, mAppIconId));
             return;
         }
 
         // Loads application icon and label based on consumer type.
         switch (mConsumerType) {
             case ConvertUtils.CONSUMER_TYPE_USER_BATTERY:
-                final BatteryEntry.NameAndIcon nameAndIconForUser =
+                final NameAndIcon nameAndIconForUser =
                         BatteryEntry.getNameAndIconFromUserId(mContext, (int) mUserId);
                 if (nameAndIconForUser != null) {
                     mAppIcon = nameAndIconForUser.mIcon;
                     mAppLabel = nameAndIconForUser.mName;
-                    sResourceCache.put(
+                    putResourceCache(
                             getKey(),
-                            new BatteryEntry.NameAndIcon(mAppLabel, mAppIcon, /* iconId= */ 0));
+                            new NameAndIcon(mAppLabel, mAppIcon, /* iconId= */ 0));
                 }
                 break;
             case ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY:
-                final BatteryEntry.NameAndIcon nameAndIconForSystem =
+                final NameAndIcon nameAndIconForSystem =
                         BatteryEntry.getNameAndIconFromPowerComponent(mContext, mComponentId);
                 if (nameAndIconForSystem != null) {
                     mAppLabel = nameAndIconForSystem.mName;
@@ -370,9 +387,8 @@ public class BatteryDiffEntry {
                         mAppIconId = nameAndIconForSystem.mIconId;
                         mAppIcon = mContext.getDrawable(nameAndIconForSystem.mIconId);
                     }
-                    sResourceCache.put(
+                    putResourceCache(
-                            getKey(),
+                            getKey(), new NameAndIcon(mAppLabel, mAppIcon, mAppIconId));
-                            new BatteryEntry.NameAndIcon(mAppLabel, mAppIcon, mAppIconId));
                 }
                 break;
             case ConvertUtils.CONSUMER_TYPE_UID_BATTERY:
@@ -384,9 +400,9 @@ public class BatteryDiffEntry {
                 // Adds badge icon into app icon for work profile.
                 mAppIcon = getBadgeIconForUser(mAppIcon);
                 if (mAppLabel != null || mAppIcon != null) {
-                    sResourceCache.put(
+                    putResourceCache(
                             getKey(),
-                            new BatteryEntry.NameAndIcon(mAppLabel, mAppIcon, /* iconId= */ 0));
+                            new NameAndIcon(mAppLabel, mAppIcon, /* iconId= */ 0));
                 }
                 break;
         }
@@ -429,7 +445,7 @@ public class BatteryDiffEntry {
         }
     }
 
-    private BatteryEntry.NameAndIcon getCache() {
+    private NameAndIcon getCache() {
         final Locale locale = Locale.getDefault();
         if (sCurrentLocale != locale) {
             Log.d(
@@ -440,7 +456,9 @@ public class BatteryDiffEntry {
             sCurrentLocale = locale;
             clearCache();
         }
-        return sResourceCache.get(getKey());
+        synchronized (sResourceCacheLock) {
+            return sResourceCache.get(getKey());
+        }
     }
 
     private void loadNameAndIconForUid() {
@@ -469,13 +487,13 @@ public class BatteryDiffEntry {
         final String[] packages = packageManager.getPackagesForUid(uid);
         // Loads special defined application label and icon if available.
         if (packages == null || packages.length == 0) {
-            final BatteryEntry.NameAndIcon nameAndIcon =
+            final NameAndIcon nameAndIcon =
                     BatteryEntry.getNameAndIconFromUid(mContext, mAppLabel, uid);
             mAppLabel = nameAndIcon.mName;
             mAppIcon = nameAndIcon.mIcon;
         }
 
-        final BatteryEntry.NameAndIcon nameAndIcon =
+        final NameAndIcon nameAndIcon =
                 BatteryEntry.loadNameAndIcon(
                         mContext, uid, /* batteryEntry= */ null, packageName, mAppLabel, mAppIcon);
         // Clears BatteryEntry internal cache since we will have another one.
@@ -544,9 +562,21 @@ public class BatteryDiffEntry {
 
     /** Clears all cache data. */
     public static void clearCache() {
-        sResourceCache.clear();
+        synchronized (sResourceCacheLock) {
-        sValidForRestriction.clear();
+            sResourceCache.clear();
-        sPackageNameAndUidCache.clear();
+        }
+        synchronized (sValidForRestrictionLock) {
+            sValidForRestriction.clear();
+        }
+        synchronized (sPackageNameAndUidCacheLock) {
+            sPackageNameAndUidCache.clear();
+        }
+    }
+
+    private static void putResourceCache(String key, NameAndIcon nameAndIcon) {
+        synchronized (sResourceCacheLock) {
+            sResourceCache.put(key, nameAndIcon);
+        }
     }
 
     private Drawable getBadgeIconForUser(Drawable icon) {