Merge "Fix ConcurrentModificationException from LocationController" into oc-dev

packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java

@@ -31,8 +31,10 @@ import android.os.Message;
 import android.os.UserHandle;
 import android.os.UserManager;
 import android.provider.Settings;
+import android.support.annotation.VisibleForTesting;
 
 import com.android.systemui.R;
+import com.android.systemui.util.Utils;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -141,7 +143,8 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio
     /**
      * Returns true if there currently exist active high power location requests.
      */
-    private boolean areActiveHighPowerLocationRequests() {
+    @VisibleForTesting
+    protected boolean areActiveHighPowerLocationRequests() {
         List<AppOpsManager.PackageOps> packages
             = mAppOpsManager.getPackagesForOps(mHighPowerRequestAppOpArray);
         // AppOpsManager can return null when there is no requested data.
@@ -205,16 +208,14 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio
         }
 
         private void locationActiveChanged() {
-            for (LocationChangeCallback cb : mSettingsChangeCallbacks) {
-                cb.onLocationActiveChanged(mAreActiveLocationRequests);
-            }
+            Utils.safeForeach(mSettingsChangeCallbacks,
+                    cb -> cb.onLocationActiveChanged(mAreActiveLocationRequests));
         }
 
         private void locationSettingsChanged() {
             boolean isEnabled = isLocationEnabled();
-            for (LocationChangeCallback cb : mSettingsChangeCallbacks) {
-                cb.onLocationSettingsChanged(isEnabled);
-            }
+            Utils.safeForeach(mSettingsChangeCallbacks,
+                    cb -> cb.onLocationSettingsChanged(isEnabled));
         }
     }
 }

packages/SystemUI/tests/AndroidManifest.xml

@@ -37,6 +37,7 @@
     <uses-permission android:name="android.permission.REQUEST_NETWORK_SCORES" />
     <uses-permission android:name="android.permission.CONTROL_VPN" />
     <uses-permission android:name="android.permission.WAKE_LOCK" />
+    <uses-permission android:name="android.permission.GET_APP_OPS_STATS" />
 
     <application>
         <uses-library android:name="android.test.runner" />

packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/LocationControllerImplTest.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the
+ * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package com.android.systemui.statusbar.policy;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+
+import android.content.Intent;
+import android.location.LocationManager;
+import android.support.test.filters.SmallTest;
+import android.testing.AndroidTestingRunner;
+import android.testing.TestableLooper;
+import android.testing.TestableLooper.RunWithLooper;
+
+import com.android.systemui.SysuiTestCase;
+import com.android.systemui.statusbar.policy.LocationController.LocationChangeCallback;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(AndroidTestingRunner.class)
+@RunWithLooper
+@SmallTest
+public class LocationControllerImplTest extends SysuiTestCase {
+
+    private LocationControllerImpl mLocationController;
+
+    @Before
+    public void setup() {
+        mLocationController = spy(new LocationControllerImpl(mContext,
+                TestableLooper.get(this).getLooper()));
+    }
+
+    @Test
+    public void testRemoveSelfActive_DoesNotCrash() {
+        LocationController.LocationChangeCallback callback = new LocationChangeCallback() {
+            @Override
+            public void onLocationActiveChanged(boolean active) {
+                mLocationController.removeCallback(this);
+            }
+        };
+        mLocationController.addCallback(callback);
+        mLocationController.addCallback(mock(LocationChangeCallback.class));
+
+        when(mLocationController.areActiveHighPowerLocationRequests()).thenReturn(false);
+        mLocationController.onReceive(mContext, new Intent(
+                LocationManager.HIGH_POWER_REQUEST_CHANGE_ACTION));
+        when(mLocationController.areActiveHighPowerLocationRequests()).thenReturn(true);
+        mLocationController.onReceive(mContext, new Intent(
+                LocationManager.HIGH_POWER_REQUEST_CHANGE_ACTION));
+
+        TestableLooper.get(this).processAllMessages();
+    }
+
+    @Test
+    public void testRemoveSelfSettings_DoesNotCrash() {
+        LocationController.LocationChangeCallback callback = new LocationChangeCallback() {
+            @Override
+            public void onLocationSettingsChanged(boolean isEnabled) {
+                mLocationController.removeCallback(this);
+            }
+        };
+        mLocationController.addCallback(callback);
+        mLocationController.addCallback(mock(LocationChangeCallback.class));
+
+        TestableLooper.get(this).processAllMessages();
+    }
+}