From c9d6847270ec931c2abc1d07ab273bebd9fcfa08 Mon Sep 17 00:00:00 2001
From: Caitlin Shkuratov <caitlinshk@google.com>
Date: Mon, 7 Aug 2023 20:59:57 +0000
Subject: [PATCH] [Status Bar] Stop using Utils.safeForeach in
 LocationControllerImpl.

Fixes: 286841705
Test: verify location tile still gets location updates
Test: verify location icon in status bar still appears when location is
being used

Change-Id: Iff4672222571f8019adf9d84e1fb681422a1603a
---
 .../policy/LocationControllerImpl.java        | 21 +++++++++++++------
 .../src/com/android/systemui/util/Utils.java  |  4 ++++
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java
index f8c36dcc90a17..518a9b3f61ec9 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java
@@ -55,7 +55,6 @@ import com.android.systemui.dagger.qualifiers.Background;
 import com.android.systemui.dagger.qualifiers.Main;
 import com.android.systemui.settings.UserTracker;
 import com.android.systemui.util.DeviceConfigProxy;
-import com.android.systemui.util.Utils;
 import com.android.systemui.util.settings.SecureSettings;
 
 import java.util.ArrayList;
@@ -362,7 +361,8 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio
         private static final int MSG_ADD_CALLBACK = 3;
         private static final int MSG_REMOVE_CALLBACK = 4;
 
-        private ArrayList<LocationChangeCallback> mSettingsChangeCallbacks = new ArrayList<>();
+        private final ArrayList<LocationChangeCallback> mSettingsChangeCallbacks =
+                new ArrayList<>();
 
         H(Looper looper) {
             super(looper);
@@ -388,14 +388,23 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio
         }
 
         private void locationActiveChanged() {
-            Utils.safeForeach(mSettingsChangeCallbacks,
-                    cb -> cb.onLocationActiveChanged(mAreActiveLocationRequests));
+            synchronized (mSettingsChangeCallbacks) {
+                final int n = mSettingsChangeCallbacks.size();
+                for (int i = 0; i < n; i++) {
+                    mSettingsChangeCallbacks.get(i)
+                            .onLocationActiveChanged(mAreActiveLocationRequests);
+                }
+            }
         }
 
         private void locationSettingsChanged() {
             boolean isEnabled = isLocationEnabled();
-            Utils.safeForeach(mSettingsChangeCallbacks,
-                    cb -> cb.onLocationSettingsChanged(isEnabled));
+            synchronized (mSettingsChangeCallbacks) {
+                final int n = mSettingsChangeCallbacks.size();
+                for (int i = 0; i < n; i++) {
+                    mSettingsChangeCallbacks.get(i).onLocationSettingsChanged(isEnabled);
+                }
+            }
         }
     }
 
diff --git a/packages/SystemUI/src/com/android/systemui/util/Utils.java b/packages/SystemUI/src/com/android/systemui/util/Utils.java
index c2727fc324659..e0daa0706b865 100644
--- a/packages/SystemUI/src/com/android/systemui/util/Utils.java
+++ b/packages/SystemUI/src/com/android/systemui/util/Utils.java
@@ -37,6 +37,10 @@ public class Utils {
     /**
      * Allows lambda iteration over a list. It is done in reverse order so it is safe
      * to add or remove items during the iteration.  Skips over null items.
+     *
+     * @deprecated According to b/286841705, this is *not* safe: If an item is removed from the
+     *   list, then list.get(i) could throw an IndexOutOfBoundsException. This method should not be
+     *   used; try using `synchronized` or making a copy of the list instead.
      */
     public static <T> void safeForeach(List<T> list, Consumer<T> c) {
         for (int i = list.size() - 1; i >= 0; i--) {